[PATCH 0/2] KVM: arm64: Minor improvements to RAZ register handling

[PATCH 0/2] KVM: arm64: Minor improvements to RAZ register handling

[Alexandru Elisei]

What sparked these two small patches is the series that fixed the PMU reset
values and their visibility from userspace, more specifically the
discussion around the patch that removed the PMSWINC_EL0 shadow register
[1].

The patches are straightforward cleanups without any changes in
functionality.

Tested on a rockpro64, by running kvm-unit-tests under qemu.

[1] https://www.spinics.net/lists/kvm-arm/msg47976.html

Alexandru Elisei (2):
  KVM: arm64: Return early from read_id_reg() if register is RAZ
  KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

 arch/arm64/kvm/sys_regs.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

-- 
2.33.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 0/2] KVM: arm64: Minor improvements to RAZ register handling

[Alexandru Elisei]

What sparked these two small patches is the series that fixed the PMU reset
values and their visibility from userspace, more specifically the
discussion around the patch that removed the PMSWINC_EL0 shadow register
[1].

The patches are straightforward cleanups without any changes in
functionality.

Tested on a rockpro64, by running kvm-unit-tests under qemu.

[1] https://www.spinics.net/lists/kvm-arm/msg47976.html

Alexandru Elisei (2):
  KVM: arm64: Return early from read_id_reg() if register is RAZ
  KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

 arch/arm64/kvm/sys_regs.c | 18 ++++++++++++++++--
 1 file changed, 16 insertions(+), 2 deletions(-)

-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 1/2] KVM: arm64: Return early from read_id_reg() if register is RAZ

[Alexandru Elisei]

If read_id_reg() is called for an ID register which is Read-As-Zero
(RAZ), it initializes the return value to zero, then goes through a list
of registers which require special handling.

By not returning as soon as it tests if the register is RAZ, it creates
the opportunity for bugs, if a patch changes a register to RAZ (like has
happened with PMSWINC_EL0 in commit 11663111cd49), but doesn't remove the
special handling from read_id_reg(); or if a register is RAZ in certain
situations, and readable in others.

Return early as to make it impossible for a RAZ register to be anything
other than zero.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/sys_regs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1d46e185f31e..4adda8bf3168 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1064,7 +1064,12 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		struct sys_reg_desc const *r, bool raz)
 {
 	u32 id = reg_to_encoding(r);
-	u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
+	u64 val;
+
+	if (raz)
+		return 0;
+
+	val = read_sanitised_ftr_reg(id);
 
 	switch (id) {
 	case SYS_ID_AA64PFR0_EL1:
-- 
2.33.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 1/2] KVM: arm64: Return early from read_id_reg() if register is RAZ

[Alexandru Elisei]

If read_id_reg() is called for an ID register which is Read-As-Zero
(RAZ), it initializes the return value to zero, then goes through a list
of registers which require special handling.

By not returning as soon as it tests if the register is RAZ, it creates
the opportunity for bugs, if a patch changes a register to RAZ (like has
happened with PMSWINC_EL0 in commit 11663111cd49), but doesn't remove the
special handling from read_id_reg(); or if a register is RAZ in certain
situations, and readable in others.

Return early as to make it impossible for a RAZ register to be anything
other than zero.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/sys_regs.c | 7 ++++++-
 1 file changed, 6 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 1d46e185f31e..4adda8bf3168 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1064,7 +1064,12 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
 		struct sys_reg_desc const *r, bool raz)
 {
 	u32 id = reg_to_encoding(r);
-	u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
+	u64 val;
+
+	if (raz)
+		return 0;
+
+	val = read_sanitised_ftr_reg(id);
 
 	switch (id) {
 	case SYS_ID_AA64PFR0_EL1:
-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 1/2] KVM: arm64: Return early from read_id_reg() if register is RAZ

[Andrew Jones]

On Mon, Sep 27, 2021 at 01:49:10PM +0100, Alexandru Elisei wrote:
> If read_id_reg() is called for an ID register which is Read-As-Zero
> (RAZ), it initializes the return value to zero, then goes through a list
> of registers which require special handling.
> 
> By not returning as soon as it tests if the register is RAZ, it creates
> the opportunity for bugs, if a patch changes a register to RAZ (like has
> happened with PMSWINC_EL0 in commit 11663111cd49), but doesn't remove the
> special handling from read_id_reg(); or if a register is RAZ in certain
> situations, and readable in others.
> 
> Return early as to make it impossible for a RAZ register to be anything
> other than zero.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arch/arm64/kvm/sys_regs.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1d46e185f31e..4adda8bf3168 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1064,7 +1064,12 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
>  		struct sys_reg_desc const *r, bool raz)
>  {
>  	u32 id = reg_to_encoding(r);
> -	u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
> +	u64 val;
> +
> +	if (raz)
> +		return 0;
> +
> +	val = read_sanitised_ftr_reg(id);
>  
>  	switch (id) {
>  	case SYS_ID_AA64PFR0_EL1:
> -- 
> 2.33.0
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
 
Reviewed-by: Andrew Jones <drjones@redhat.com>

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 1/2] KVM: arm64: Return early from read_id_reg() if register is RAZ

[Andrew Jones]

On Mon, Sep 27, 2021 at 01:49:10PM +0100, Alexandru Elisei wrote:
> If read_id_reg() is called for an ID register which is Read-As-Zero
> (RAZ), it initializes the return value to zero, then goes through a list
> of registers which require special handling.
> 
> By not returning as soon as it tests if the register is RAZ, it creates
> the opportunity for bugs, if a patch changes a register to RAZ (like has
> happened with PMSWINC_EL0 in commit 11663111cd49), but doesn't remove the
> special handling from read_id_reg(); or if a register is RAZ in certain
> situations, and readable in others.
> 
> Return early as to make it impossible for a RAZ register to be anything
> other than zero.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arch/arm64/kvm/sys_regs.c | 7 ++++++-
>  1 file changed, 6 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 1d46e185f31e..4adda8bf3168 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1064,7 +1064,12 @@ static u64 read_id_reg(const struct kvm_vcpu *vcpu,
>  		struct sys_reg_desc const *r, bool raz)
>  {
>  	u32 id = reg_to_encoding(r);
> -	u64 val = raz ? 0 : read_sanitised_ftr_reg(id);
> +	u64 val;
> +
> +	if (raz)
> +		return 0;
> +
> +	val = read_sanitised_ftr_reg(id);
>  
>  	switch (id) {
>  	case SYS_ID_AA64PFR0_EL1:
> -- 
> 2.33.0
> 
> _______________________________________________
> kvmarm mailing list
> kvmarm@lists.cs.columbia.edu
> https://lists.cs.columbia.edu/mailman/listinfo/kvmarm
>
 
Reviewed-by: Andrew Jones <drjones@redhat.com>


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

[PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Alexandru Elisei]

PMSWINC_EL0 is a write-only register and was initially part of the VCPU
register state, but was later removed in commit 7a3ba3095a32 ("KVM:
arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
register was kept accessible from userspace as Read-As-Zero (RAZ).

The read function that is used to handle userspace reads of this
register is get_raz_id_reg(), which, while technically correct, as it
returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
register as the function name suggests.

Add a new function, get_raz_reg(), to use it as the accessor for
PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
of registers.

No functional change intended.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 4adda8bf3168..1be827740f87 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 	return __set_id_reg(vcpu, rd, uaddr, true);
 }
 
+static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+		       const struct kvm_one_reg *reg, void __user *uaddr)
+{
+	const u64 id = sys_reg_to_index(rd);
+	const u64 val = 0;
+
+	return reg_to_user(uaddr, &val, id);
+}
+
 static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 		      const struct kvm_one_reg *reg, void __user *uaddr)
 {
@@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	 * previously (and pointlessly) advertised in the past...
 	 */
 	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
-	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
+	  .get_user = get_raz_reg, .set_user = set_wi_reg,
 	  .access = access_pmswinc, .reset = NULL },
 	{ PMU_SYS_REG(SYS_PMSELR_EL0),
 	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
-- 
2.33.0

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

[PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Alexandru Elisei]

PMSWINC_EL0 is a write-only register and was initially part of the VCPU
register state, but was later removed in commit 7a3ba3095a32 ("KVM:
arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
register was kept accessible from userspace as Read-As-Zero (RAZ).

The read function that is used to handle userspace reads of this
register is get_raz_id_reg(), which, while technically correct, as it
returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
register as the function name suggests.

Add a new function, get_raz_reg(), to use it as the accessor for
PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
of registers.

No functional change intended.

Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
---
 arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
index 4adda8bf3168..1be827740f87 100644
--- a/arch/arm64/kvm/sys_regs.c
+++ b/arch/arm64/kvm/sys_regs.c
@@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 	return __set_id_reg(vcpu, rd, uaddr, true);
 }
 
+static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
+		       const struct kvm_one_reg *reg, void __user *uaddr)
+{
+	const u64 id = sys_reg_to_index(rd);
+	const u64 val = 0;
+
+	return reg_to_user(uaddr, &val, id);
+}
+
 static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
 		      const struct kvm_one_reg *reg, void __user *uaddr)
 {
@@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
 	 * previously (and pointlessly) advertised in the past...
 	 */
 	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
-	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
+	  .get_user = get_raz_reg, .set_user = set_wi_reg,
 	  .access = access_pmswinc, .reset = NULL },
 	{ PMU_SYS_REG(SYS_PMSELR_EL0),
 	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
-- 
2.33.0


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Andrew Jones]

On Mon, Sep 27, 2021 at 01:49:11PM +0100, Alexandru Elisei wrote:
> PMSWINC_EL0 is a write-only register and was initially part of the VCPU
> register state, but was later removed in commit 7a3ba3095a32 ("KVM:
> arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
> register was kept accessible from userspace as Read-As-Zero (RAZ).
> 
> The read function that is used to handle userspace reads of this
> register is get_raz_id_reg(), which, while technically correct, as it
> returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
> register as the function name suggests.
> 
> Add a new function, get_raz_reg(), to use it as the accessor for
> PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
> of registers.
> 
> No functional change intended.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 4adda8bf3168..1be827740f87 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  	return __set_id_reg(vcpu, rd, uaddr, true);
>  }
>  
> +static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> +		       const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> +	const u64 id = sys_reg_to_index(rd);
> +	const u64 val = 0;
> +
> +	return reg_to_user(uaddr, &val, id);
> +}
> +
>  static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  		      const struct kvm_one_reg *reg, void __user *uaddr)
>  {
> @@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	 * previously (and pointlessly) advertised in the past...
>  	 */
>  	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
> -	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
> +	  .get_user = get_raz_reg, .set_user = set_wi_reg,
>  	  .access = access_pmswinc, .reset = NULL },
>  	{ PMU_SYS_REG(SYS_PMSELR_EL0),
>  	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
> -- 
> 2.33.0
>

What about replacing get_raz_id_reg() with this new function? Do really need
both?

Thanks,
drew

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Andrew Jones]

On Mon, Sep 27, 2021 at 01:49:11PM +0100, Alexandru Elisei wrote:
> PMSWINC_EL0 is a write-only register and was initially part of the VCPU
> register state, but was later removed in commit 7a3ba3095a32 ("KVM:
> arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
> register was kept accessible from userspace as Read-As-Zero (RAZ).
> 
> The read function that is used to handle userspace reads of this
> register is get_raz_id_reg(), which, while technically correct, as it
> returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
> register as the function name suggests.
> 
> Add a new function, get_raz_reg(), to use it as the accessor for
> PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
> of registers.
> 
> No functional change intended.
> 
> Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> ---
>  arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
> 
> diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> index 4adda8bf3168..1be827740f87 100644
> --- a/arch/arm64/kvm/sys_regs.c
> +++ b/arch/arm64/kvm/sys_regs.c
> @@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  	return __set_id_reg(vcpu, rd, uaddr, true);
>  }
>  
> +static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> +		       const struct kvm_one_reg *reg, void __user *uaddr)
> +{
> +	const u64 id = sys_reg_to_index(rd);
> +	const u64 val = 0;
> +
> +	return reg_to_user(uaddr, &val, id);
> +}
> +
>  static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
>  		      const struct kvm_one_reg *reg, void __user *uaddr)
>  {
> @@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
>  	 * previously (and pointlessly) advertised in the past...
>  	 */
>  	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
> -	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
> +	  .get_user = get_raz_reg, .set_user = set_wi_reg,
>  	  .access = access_pmswinc, .reset = NULL },
>  	{ PMU_SYS_REG(SYS_PMSELR_EL0),
>  	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
> -- 
> 2.33.0
>

What about replacing get_raz_id_reg() with this new function? Do really need
both?

Thanks,
drew


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Alexandru Elisei]

Hi Drew,

Thank you for the review!

On Thu, Sep 30, 2021 at 03:29:15PM +0200, Andrew Jones wrote:
> On Mon, Sep 27, 2021 at 01:49:11PM +0100, Alexandru Elisei wrote:
> > PMSWINC_EL0 is a write-only register and was initially part of the VCPU
> > register state, but was later removed in commit 7a3ba3095a32 ("KVM:
> > arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
> > register was kept accessible from userspace as Read-As-Zero (RAZ).
> > 
> > The read function that is used to handle userspace reads of this
> > register is get_raz_id_reg(), which, while technically correct, as it
> > returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
> > register as the function name suggests.
> > 
> > Add a new function, get_raz_reg(), to use it as the accessor for
> > PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
> > of registers.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 4adda8bf3168..1be827740f87 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> >  	return __set_id_reg(vcpu, rd, uaddr, true);
> >  }
> >  
> > +static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > +		       const struct kvm_one_reg *reg, void __user *uaddr)
> > +{
> > +	const u64 id = sys_reg_to_index(rd);
> > +	const u64 val = 0;
> > +
> > +	return reg_to_user(uaddr, &val, id);
> > +}
> > +
> >  static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> >  		      const struct kvm_one_reg *reg, void __user *uaddr)
> >  {
> > @@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> >  	 * previously (and pointlessly) advertised in the past...
> >  	 */
> >  	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
> > -	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
> > +	  .get_user = get_raz_reg, .set_user = set_wi_reg,
> >  	  .access = access_pmswinc, .reset = NULL },
> >  	{ PMU_SYS_REG(SYS_PMSELR_EL0),
> >  	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
> > -- 
> > 2.33.0
> >
> 
> What about replacing get_raz_id_reg() with this new function? Do really need
> both?

I thought about that when writing this patch. I ultimately decided against it
because changing the get_user accessor to be get_raz_reg() instead of
get_raz_id_reg() would break the symmetry with set_user, which needs to stay
set_raz_id_reg(), and cannot be substituted with set_wi_reg() because that would
be a change in behaviour (set_raz_id_reg() checks that val == 0, set_wi_reg()
doesn't).

I do agree that get_raz_id_reg() does the exact same thing as get_raz_reg(), but
in a more roundabout manner. So if you still feel that I should use
get_raz_reg() instead, I'll do that for the next iteration of the series. What
do you think?

Thanks,
Alex

> 
> Thanks,
> drew
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Alexandru Elisei]

Hi Drew,

Thank you for the review!

On Thu, Sep 30, 2021 at 03:29:15PM +0200, Andrew Jones wrote:
> On Mon, Sep 27, 2021 at 01:49:11PM +0100, Alexandru Elisei wrote:
> > PMSWINC_EL0 is a write-only register and was initially part of the VCPU
> > register state, but was later removed in commit 7a3ba3095a32 ("KVM:
> > arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
> > register was kept accessible from userspace as Read-As-Zero (RAZ).
> > 
> > The read function that is used to handle userspace reads of this
> > register is get_raz_id_reg(), which, while technically correct, as it
> > returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
> > register as the function name suggests.
> > 
> > Add a new function, get_raz_reg(), to use it as the accessor for
> > PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
> > of registers.
> > 
> > No functional change intended.
> > 
> > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > ---
> >  arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
> >  1 file changed, 10 insertions(+), 1 deletion(-)
> > 
> > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > index 4adda8bf3168..1be827740f87 100644
> > --- a/arch/arm64/kvm/sys_regs.c
> > +++ b/arch/arm64/kvm/sys_regs.c
> > @@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> >  	return __set_id_reg(vcpu, rd, uaddr, true);
> >  }
> >  
> > +static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > +		       const struct kvm_one_reg *reg, void __user *uaddr)
> > +{
> > +	const u64 id = sys_reg_to_index(rd);
> > +	const u64 val = 0;
> > +
> > +	return reg_to_user(uaddr, &val, id);
> > +}
> > +
> >  static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> >  		      const struct kvm_one_reg *reg, void __user *uaddr)
> >  {
> > @@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> >  	 * previously (and pointlessly) advertised in the past...
> >  	 */
> >  	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
> > -	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
> > +	  .get_user = get_raz_reg, .set_user = set_wi_reg,
> >  	  .access = access_pmswinc, .reset = NULL },
> >  	{ PMU_SYS_REG(SYS_PMSELR_EL0),
> >  	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
> > -- 
> > 2.33.0
> >
> 
> What about replacing get_raz_id_reg() with this new function? Do really need
> both?

I thought about that when writing this patch. I ultimately decided against it
because changing the get_user accessor to be get_raz_reg() instead of
get_raz_id_reg() would break the symmetry with set_user, which needs to stay
set_raz_id_reg(), and cannot be substituted with set_wi_reg() because that would
be a change in behaviour (set_raz_id_reg() checks that val == 0, set_wi_reg()
doesn't).

I do agree that get_raz_id_reg() does the exact same thing as get_raz_reg(), but
in a more roundabout manner. So if you still feel that I should use
get_raz_reg() instead, I'll do that for the next iteration of the series. What
do you think?

Thanks,
Alex

> 
> Thanks,
> drew
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Andrew Jones]

On Wed, Oct 06, 2021 at 03:49:19PM +0100, Alexandru Elisei wrote:
> Hi Drew,
> 
> Thank you for the review!
> 
> On Thu, Sep 30, 2021 at 03:29:15PM +0200, Andrew Jones wrote:
> > On Mon, Sep 27, 2021 at 01:49:11PM +0100, Alexandru Elisei wrote:
> > > PMSWINC_EL0 is a write-only register and was initially part of the VCPU
> > > register state, but was later removed in commit 7a3ba3095a32 ("KVM:
> > > arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
> > > register was kept accessible from userspace as Read-As-Zero (RAZ).
> > > 
> > > The read function that is used to handle userspace reads of this
> > > register is get_raz_id_reg(), which, while technically correct, as it
> > > returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
> > > register as the function name suggests.
> > > 
> > > Add a new function, get_raz_reg(), to use it as the accessor for
> > > PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
> > > of registers.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > ---
> > >  arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > index 4adda8bf3168..1be827740f87 100644
> > > --- a/arch/arm64/kvm/sys_regs.c
> > > +++ b/arch/arm64/kvm/sys_regs.c
> > > @@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > >  	return __set_id_reg(vcpu, rd, uaddr, true);
> > >  }
> > >  
> > > +static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > > +		       const struct kvm_one_reg *reg, void __user *uaddr)
> > > +{
> > > +	const u64 id = sys_reg_to_index(rd);
> > > +	const u64 val = 0;
> > > +
> > > +	return reg_to_user(uaddr, &val, id);
> > > +}
> > > +
> > >  static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > >  		      const struct kvm_one_reg *reg, void __user *uaddr)
> > >  {
> > > @@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> > >  	 * previously (and pointlessly) advertised in the past...
> > >  	 */
> > >  	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
> > > -	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
> > > +	  .get_user = get_raz_reg, .set_user = set_wi_reg,
> > >  	  .access = access_pmswinc, .reset = NULL },
> > >  	{ PMU_SYS_REG(SYS_PMSELR_EL0),
> > >  	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
> > > -- 
> > > 2.33.0
> > >
> > 
> > What about replacing get_raz_id_reg() with this new function? Do really need
> > both?
> 
> I thought about that when writing this patch. I ultimately decided against it
> because changing the get_user accessor to be get_raz_reg() instead of
> get_raz_id_reg() would break the symmetry with set_user, which needs to stay
> set_raz_id_reg(), and cannot be substituted with set_wi_reg() because that would
> be a change in behaviour (set_raz_id_reg() checks that val == 0, set_wi_reg()
> doesn't).
> 
> I do agree that get_raz_id_reg() does the exact same thing as get_raz_reg(), but
> in a more roundabout manner. So if you still feel that I should use
> get_raz_reg() instead, I'll do that for the next iteration of the series. What
> do you think?

I'd prefer we avoid maintaining two implementations of the same
functionality. If we want to keep the symmetry with set_raz_id_reg,
then we could implement get_raz_id_reg as 'return get_raz_reg()'.

Thanks,
drew

_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Andrew Jones]

On Wed, Oct 06, 2021 at 03:49:19PM +0100, Alexandru Elisei wrote:
> Hi Drew,
> 
> Thank you for the review!
> 
> On Thu, Sep 30, 2021 at 03:29:15PM +0200, Andrew Jones wrote:
> > On Mon, Sep 27, 2021 at 01:49:11PM +0100, Alexandru Elisei wrote:
> > > PMSWINC_EL0 is a write-only register and was initially part of the VCPU
> > > register state, but was later removed in commit 7a3ba3095a32 ("KVM:
> > > arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
> > > register was kept accessible from userspace as Read-As-Zero (RAZ).
> > > 
> > > The read function that is used to handle userspace reads of this
> > > register is get_raz_id_reg(), which, while technically correct, as it
> > > returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
> > > register as the function name suggests.
> > > 
> > > Add a new function, get_raz_reg(), to use it as the accessor for
> > > PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
> > > of registers.
> > > 
> > > No functional change intended.
> > > 
> > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > ---
> > >  arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
> > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > 
> > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > index 4adda8bf3168..1be827740f87 100644
> > > --- a/arch/arm64/kvm/sys_regs.c
> > > +++ b/arch/arm64/kvm/sys_regs.c
> > > @@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > >  	return __set_id_reg(vcpu, rd, uaddr, true);
> > >  }
> > >  
> > > +static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > > +		       const struct kvm_one_reg *reg, void __user *uaddr)
> > > +{
> > > +	const u64 id = sys_reg_to_index(rd);
> > > +	const u64 val = 0;
> > > +
> > > +	return reg_to_user(uaddr, &val, id);
> > > +}
> > > +
> > >  static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > >  		      const struct kvm_one_reg *reg, void __user *uaddr)
> > >  {
> > > @@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> > >  	 * previously (and pointlessly) advertised in the past...
> > >  	 */
> > >  	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
> > > -	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
> > > +	  .get_user = get_raz_reg, .set_user = set_wi_reg,
> > >  	  .access = access_pmswinc, .reset = NULL },
> > >  	{ PMU_SYS_REG(SYS_PMSELR_EL0),
> > >  	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
> > > -- 
> > > 2.33.0
> > >
> > 
> > What about replacing get_raz_id_reg() with this new function? Do really need
> > both?
> 
> I thought about that when writing this patch. I ultimately decided against it
> because changing the get_user accessor to be get_raz_reg() instead of
> get_raz_id_reg() would break the symmetry with set_user, which needs to stay
> set_raz_id_reg(), and cannot be substituted with set_wi_reg() because that would
> be a change in behaviour (set_raz_id_reg() checks that val == 0, set_wi_reg()
> doesn't).
> 
> I do agree that get_raz_id_reg() does the exact same thing as get_raz_reg(), but
> in a more roundabout manner. So if you still feel that I should use
> get_raz_reg() instead, I'll do that for the next iteration of the series. What
> do you think?

I'd prefer we avoid maintaining two implementations of the same
functionality. If we want to keep the symmetry with set_raz_id_reg,
then we could implement get_raz_id_reg as 'return get_raz_reg()'.

Thanks,
drew


_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Alexandru Elisei]

Hi Drew,

On Wed, Oct 06, 2021 at 05:23:02PM +0200, Andrew Jones wrote:
> On Wed, Oct 06, 2021 at 03:49:19PM +0100, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > Thank you for the review!
> > 
> > On Thu, Sep 30, 2021 at 03:29:15PM +0200, Andrew Jones wrote:
> > > On Mon, Sep 27, 2021 at 01:49:11PM +0100, Alexandru Elisei wrote:
> > > > PMSWINC_EL0 is a write-only register and was initially part of the VCPU
> > > > register state, but was later removed in commit 7a3ba3095a32 ("KVM:
> > > > arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
> > > > register was kept accessible from userspace as Read-As-Zero (RAZ).
> > > > 
> > > > The read function that is used to handle userspace reads of this
> > > > register is get_raz_id_reg(), which, while technically correct, as it
> > > > returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
> > > > register as the function name suggests.
> > > > 
> > > > Add a new function, get_raz_reg(), to use it as the accessor for
> > > > PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
> > > > of registers.
> > > > 
> > > > No functional change intended.
> > > > 
> > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > ---
> > > >  arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
> > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > > index 4adda8bf3168..1be827740f87 100644
> > > > --- a/arch/arm64/kvm/sys_regs.c
> > > > +++ b/arch/arm64/kvm/sys_regs.c
> > > > @@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > > >  	return __set_id_reg(vcpu, rd, uaddr, true);
> > > >  }
> > > >  
> > > > +static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > > > +		       const struct kvm_one_reg *reg, void __user *uaddr)
> > > > +{
> > > > +	const u64 id = sys_reg_to_index(rd);
> > > > +	const u64 val = 0;
> > > > +
> > > > +	return reg_to_user(uaddr, &val, id);
> > > > +}
> > > > +
> > > >  static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > > >  		      const struct kvm_one_reg *reg, void __user *uaddr)
> > > >  {
> > > > @@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> > > >  	 * previously (and pointlessly) advertised in the past...
> > > >  	 */
> > > >  	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
> > > > -	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
> > > > +	  .get_user = get_raz_reg, .set_user = set_wi_reg,
> > > >  	  .access = access_pmswinc, .reset = NULL },
> > > >  	{ PMU_SYS_REG(SYS_PMSELR_EL0),
> > > >  	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
> > > > -- 
> > > > 2.33.0
> > > >
> > > 
> > > What about replacing get_raz_id_reg() with this new function? Do really need
> > > both?
> > 
> > I thought about that when writing this patch. I ultimately decided against it
> > because changing the get_user accessor to be get_raz_reg() instead of
> > get_raz_id_reg() would break the symmetry with set_user, which needs to stay
> > set_raz_id_reg(), and cannot be substituted with set_wi_reg() because that would
> > be a change in behaviour (set_raz_id_reg() checks that val == 0, set_wi_reg()
> > doesn't).
> > 
> > I do agree that get_raz_id_reg() does the exact same thing as get_raz_reg(), but
> > in a more roundabout manner. So if you still feel that I should use
> > get_raz_reg() instead, I'll do that for the next iteration of the series. What
> > do you think?
> 
> I'd prefer we avoid maintaining two implementations of the same
> functionality. If we want to keep the symmetry with set_raz_id_reg,
> then we could implement get_raz_id_reg as 'return get_raz_reg()'.

Agreed, I'll replace get_raz_id_reg() with get_raz_reg().

Thanks,
Alex

> 
> Thanks,
> drew
> 
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 2/2] KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0

[Alexandru Elisei]

Hi Drew,

On Wed, Oct 06, 2021 at 05:23:02PM +0200, Andrew Jones wrote:
> On Wed, Oct 06, 2021 at 03:49:19PM +0100, Alexandru Elisei wrote:
> > Hi Drew,
> > 
> > Thank you for the review!
> > 
> > On Thu, Sep 30, 2021 at 03:29:15PM +0200, Andrew Jones wrote:
> > > On Mon, Sep 27, 2021 at 01:49:11PM +0100, Alexandru Elisei wrote:
> > > > PMSWINC_EL0 is a write-only register and was initially part of the VCPU
> > > > register state, but was later removed in commit 7a3ba3095a32 ("KVM:
> > > > arm64: Remove PMSWINC_EL0 shadow register"). To prevent regressions, the
> > > > register was kept accessible from userspace as Read-As-Zero (RAZ).
> > > > 
> > > > The read function that is used to handle userspace reads of this
> > > > register is get_raz_id_reg(), which, while technically correct, as it
> > > > returns 0, it is not semantically correct, as PMSWINC_EL0 is not an ID
> > > > register as the function name suggests.
> > > > 
> > > > Add a new function, get_raz_reg(), to use it as the accessor for
> > > > PMSWINC_EL0, as to not conflate get_raz_id_reg() to handle other types
> > > > of registers.
> > > > 
> > > > No functional change intended.
> > > > 
> > > > Signed-off-by: Alexandru Elisei <alexandru.elisei@arm.com>
> > > > ---
> > > >  arch/arm64/kvm/sys_regs.c | 11 ++++++++++-
> > > >  1 file changed, 10 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/arch/arm64/kvm/sys_regs.c b/arch/arm64/kvm/sys_regs.c
> > > > index 4adda8bf3168..1be827740f87 100644
> > > > --- a/arch/arm64/kvm/sys_regs.c
> > > > +++ b/arch/arm64/kvm/sys_regs.c
> > > > @@ -1285,6 +1285,15 @@ static int set_raz_id_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > > >  	return __set_id_reg(vcpu, rd, uaddr, true);
> > > >  }
> > > >  
> > > > +static int get_raz_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > > > +		       const struct kvm_one_reg *reg, void __user *uaddr)
> > > > +{
> > > > +	const u64 id = sys_reg_to_index(rd);
> > > > +	const u64 val = 0;
> > > > +
> > > > +	return reg_to_user(uaddr, &val, id);
> > > > +}
> > > > +
> > > >  static int set_wi_reg(struct kvm_vcpu *vcpu, const struct sys_reg_desc *rd,
> > > >  		      const struct kvm_one_reg *reg, void __user *uaddr)
> > > >  {
> > > > @@ -1647,7 +1656,7 @@ static const struct sys_reg_desc sys_reg_descs[] = {
> > > >  	 * previously (and pointlessly) advertised in the past...
> > > >  	 */
> > > >  	{ PMU_SYS_REG(SYS_PMSWINC_EL0),
> > > > -	  .get_user = get_raz_id_reg, .set_user = set_wi_reg,
> > > > +	  .get_user = get_raz_reg, .set_user = set_wi_reg,
> > > >  	  .access = access_pmswinc, .reset = NULL },
> > > >  	{ PMU_SYS_REG(SYS_PMSELR_EL0),
> > > >  	  .access = access_pmselr, .reset = reset_pmselr, .reg = PMSELR_EL0 },
> > > > -- 
> > > > 2.33.0
> > > >
> > > 
> > > What about replacing get_raz_id_reg() with this new function? Do really need
> > > both?
> > 
> > I thought about that when writing this patch. I ultimately decided against it
> > because changing the get_user accessor to be get_raz_reg() instead of
> > get_raz_id_reg() would break the symmetry with set_user, which needs to stay
> > set_raz_id_reg(), and cannot be substituted with set_wi_reg() because that would
> > be a change in behaviour (set_raz_id_reg() checks that val == 0, set_wi_reg()
> > doesn't).
> > 
> > I do agree that get_raz_id_reg() does the exact same thing as get_raz_reg(), but
> > in a more roundabout manner. So if you still feel that I should use
> > get_raz_reg() instead, I'll do that for the next iteration of the series. What
> > do you think?
> 
> I'd prefer we avoid maintaining two implementations of the same
> functionality. If we want to keep the symmetry with set_raz_id_reg,
> then we could implement get_raz_id_reg as 'return get_raz_reg()'.

Agreed, I'll replace get_raz_id_reg() with get_raz_reg().

Thanks,
Alex

> 
> Thanks,
> drew
> 

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel

Re: [PATCH 0/2] KVM: arm64: Minor improvements to RAZ register handling

[Marc Zyngier]

Hi Alexandru,

On Mon, 27 Sep 2021 13:49:09 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> What sparked these two small patches is the series that fixed the PMU reset
> values and their visibility from userspace, more specifically the
> discussion around the patch that removed the PMSWINC_EL0 shadow register
> [1].
> 
> The patches are straightforward cleanups without any changes in
> functionality.
> 
> Tested on a rockpro64, by running kvm-unit-tests under qemu.
> 
> [1] https://www.spinics.net/lists/kvm-arm/msg47976.html
> 
> Alexandru Elisei (2):
>   KVM: arm64: Return early from read_id_reg() if register is RAZ
>   KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0
> 
>  arch/arm64/kvm/sys_regs.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 

If you are going to respin this one, now would be the time! :-)

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.
_______________________________________________
kvmarm mailing list
kvmarm@lists.cs.columbia.edu
https://lists.cs.columbia.edu/mailman/listinfo/kvmarm

Re: [PATCH 0/2] KVM: arm64: Minor improvements to RAZ register handling

[Marc Zyngier]

Hi Alexandru,

On Mon, 27 Sep 2021 13:49:09 +0100,
Alexandru Elisei <alexandru.elisei@arm.com> wrote:
> 
> What sparked these two small patches is the series that fixed the PMU reset
> values and their visibility from userspace, more specifically the
> discussion around the patch that removed the PMSWINC_EL0 shadow register
> [1].
> 
> The patches are straightforward cleanups without any changes in
> functionality.
> 
> Tested on a rockpro64, by running kvm-unit-tests under qemu.
> 
> [1] https://www.spinics.net/lists/kvm-arm/msg47976.html
> 
> Alexandru Elisei (2):
>   KVM: arm64: Return early from read_id_reg() if register is RAZ
>   KVM: arm64: Use get_raz_reg() for userspace reads of PMSWINC_EL0
> 
>  arch/arm64/kvm/sys_regs.c | 18 ++++++++++++++++--
>  1 file changed, 16 insertions(+), 2 deletions(-)
> 

If you are going to respin this one, now would be the time! :-)

Thanks,

	M.

-- 
Without deviation from the norm, progress is not possible.

_______________________________________________
linux-arm-kernel mailing list
linux-arm-kernel@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/linux-arm-kernel