1. Patchew
  2. QEMU
  3. [PATCH 0/7] target/arm: Add vSPE support to KVM guest
  4. View patch

[PATCH 4/7] target/arm/kvm: spe: Unify device attr operatioin helper

Haibo Xu posted 7 patches 5 years, 6 months ago
There is a newer version of this series
[PATCH 4/7] target/arm/kvm: spe: Unify device attr operatioin helper
Posted by Haibo Xu 5 years, 6 months ago 
Rename kvm_arm_pmu_set_attr() to kvm_arm_dev_set_attr(),
So both the vPMU and vSPE device can share the same API.

Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
---
 target/arm/kvm64.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
index 1169237905..75a417d65c 100644
--- a/target/arm/kvm64.c
+++ b/target/arm/kvm64.c
@@ -398,7 +398,7 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
     return NULL;
 }
 
-static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr)
+static bool kvm_arm_dev_set_attr(CPUState *cs, struct kvm_device_attr *attr)
 {
     int err;
 
@@ -427,7 +427,7 @@ void kvm_arm_pmu_init(CPUState *cs)
     if (!ARM_CPU(cs)->has_pmu) {
         return;
     }
-    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
+    if (!kvm_arm_dev_set_attr(cs, &attr)) {
         error_report("failed to init PMU");
         abort();
     }
@@ -444,7 +444,7 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
     if (!ARM_CPU(cs)->has_pmu) {
         return;
     }
-    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
+    if (!kvm_arm_dev_set_attr(cs, &attr)) {
         error_report("failed to set irq for PMU");
         abort();
     }
-- 
2.17.1
Re: [PATCH 4/7] target/arm/kvm: spe: Unify device attr operatioin helper
Posted by Philippe Mathieu-Daudé 5 years, 6 months ago 
On 8/7/20 10:10 AM, Haibo Xu wrote:
> Rename kvm_arm_pmu_set_attr() to kvm_arm_dev_set_attr(),

Maybe rename kvm_arm_device_set_attr() to match the structure
name?

> So both the vPMU and vSPE device can share the same API.
> 
> Signed-off-by: Haibo Xu <haibo.xu@linaro.org>

Regardless, with the typo "operation" in patch subject fixed:
Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

> ---
>  target/arm/kvm64.c | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
> 
> diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> index 1169237905..75a417d65c 100644
> --- a/target/arm/kvm64.c
> +++ b/target/arm/kvm64.c
> @@ -398,7 +398,7 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
>      return NULL;
>  }
>  
> -static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> +static bool kvm_arm_dev_set_attr(CPUState *cs, struct kvm_device_attr *attr)
>  {
>      int err;
>  
> @@ -427,7 +427,7 @@ void kvm_arm_pmu_init(CPUState *cs)
>      if (!ARM_CPU(cs)->has_pmu) {
>          return;
>      }
> -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> +    if (!kvm_arm_dev_set_attr(cs, &attr)) {
>          error_report("failed to init PMU");
>          abort();
>      }
> @@ -444,7 +444,7 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
>      if (!ARM_CPU(cs)->has_pmu) {
>          return;
>      }
> -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> +    if (!kvm_arm_dev_set_attr(cs, &attr)) {
>          error_report("failed to set irq for PMU");
>          abort();
>      }
>
Re: [PATCH 4/7] target/arm/kvm: spe: Unify device attr operatioin helper
Posted by Haibo Xu 5 years, 6 months ago 
On Fri, 7 Aug 2020 at 16:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
>
> On 8/7/20 10:10 AM, Haibo Xu wrote:
> > Rename kvm_arm_pmu_set_attr() to kvm_arm_dev_set_attr(),
>
> Maybe rename kvm_arm_device_set_attr() to match the structure
> name?
>

Thanks for the review! I will update it in the next version.

> > So both the vPMU and vSPE device can share the same API.
> >
> > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
>
> Regardless, with the typo "operation" in patch subject fixed:
> Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>
> > ---
> >  target/arm/kvm64.c | 6 +++---
> >  1 file changed, 3 insertions(+), 3 deletions(-)
> >
> > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > index 1169237905..75a417d65c 100644
> > --- a/target/arm/kvm64.c
> > +++ b/target/arm/kvm64.c
> > @@ -398,7 +398,7 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
> >      return NULL;
> >  }
> >
> > -static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> > +static bool kvm_arm_dev_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> >  {
> >      int err;
> >
> > @@ -427,7 +427,7 @@ void kvm_arm_pmu_init(CPUState *cs)
> >      if (!ARM_CPU(cs)->has_pmu) {
> >          return;
> >      }
> > -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> > +    if (!kvm_arm_dev_set_attr(cs, &attr)) {
> >          error_report("failed to init PMU");
> >          abort();
> >      }
> > @@ -444,7 +444,7 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
> >      if (!ARM_CPU(cs)->has_pmu) {
> >          return;
> >      }
> > -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> > +    if (!kvm_arm_dev_set_attr(cs, &attr)) {
> >          error_report("failed to set irq for PMU");
> >          abort();
> >      }
> >
>
Re: [PATCH 4/7] target/arm/kvm: spe: Unify device attr operatioin helper
Posted by Andrew Jones 5 years, 6 months ago 
On Mon, Aug 10, 2020 at 10:48:41AM +0800, Haibo Xu wrote:
> On Fri, 7 Aug 2020 at 16:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> >
> > On 8/7/20 10:10 AM, Haibo Xu wrote:
> > > Rename kvm_arm_pmu_set_attr() to kvm_arm_dev_set_attr(),
> >
> > Maybe rename kvm_arm_device_set_attr() to match the structure
> > name?
> >
> 
> Thanks for the review! I will update it in the next version.

I've already renamed it to kvm_arm_set_device_attr() in [1]. Also, it's
not enough to just rename the function. The error messages the function
may generate have "PMU" embedded in them.

[1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg727590.html

Thanks,
drew

> 
> > > So both the vPMU and vSPE device can share the same API.
> > >
> > > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> >
> > Regardless, with the typo "operation" in patch subject fixed:
> > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> >
> > > ---
> > >  target/arm/kvm64.c | 6 +++---
> > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > >
> > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > > index 1169237905..75a417d65c 100644
> > > --- a/target/arm/kvm64.c
> > > +++ b/target/arm/kvm64.c
> > > @@ -398,7 +398,7 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
> > >      return NULL;
> > >  }
> > >
> > > -static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> > > +static bool kvm_arm_dev_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> > >  {
> > >      int err;
> > >
> > > @@ -427,7 +427,7 @@ void kvm_arm_pmu_init(CPUState *cs)
> > >      if (!ARM_CPU(cs)->has_pmu) {
> > >          return;
> > >      }
> > > -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> > > +    if (!kvm_arm_dev_set_attr(cs, &attr)) {
> > >          error_report("failed to init PMU");
> > >          abort();
> > >      }
> > > @@ -444,7 +444,7 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
> > >      if (!ARM_CPU(cs)->has_pmu) {
> > >          return;
> > >      }
> > > -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> > > +    if (!kvm_arm_dev_set_attr(cs, &attr)) {
> > >          error_report("failed to set irq for PMU");
> > >          abort();
> > >      }
> > >
> >
>
Re: [PATCH 4/7] target/arm/kvm: spe: Unify device attr operatioin helper
Posted by Haibo Xu 5 years, 6 months ago 
On Mon, 10 Aug 2020 at 18:29, Andrew Jones <drjones@redhat.com> wrote:
>
> On Mon, Aug 10, 2020 at 10:48:41AM +0800, Haibo Xu wrote:
> > On Fri, 7 Aug 2020 at 16:19, Philippe Mathieu-Daudé <f4bug@amsat.org> wrote:
> > >
> > > On 8/7/20 10:10 AM, Haibo Xu wrote:
> > > > Rename kvm_arm_pmu_set_attr() to kvm_arm_dev_set_attr(),
> > >
> > > Maybe rename kvm_arm_device_set_attr() to match the structure
> > > name?
> > >
> >
> > Thanks for the review! I will update it in the next version.
>
> I've already renamed it to kvm_arm_set_device_attr() in [1]. Also, it's
> not enough to just rename the function. The error messages the function
> may generate have "PMU" embedded in them.
>
> [1] https://www.mail-archive.com/qemu-devel@nongnu.org/msg727590.html
>
> Thanks,
> drew
>

Thanks for your review, Andrew!
Will rebase on your patches in the next version.

> >
> > > > So both the vPMU and vSPE device can share the same API.
> > > >
> > > > Signed-off-by: Haibo Xu <haibo.xu@linaro.org>
> > >
> > > Regardless, with the typo "operation" in patch subject fixed:
> > > Reviewed-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> > >
> > > > ---
> > > >  target/arm/kvm64.c | 6 +++---
> > > >  1 file changed, 3 insertions(+), 3 deletions(-)
> > > >
> > > > diff --git a/target/arm/kvm64.c b/target/arm/kvm64.c
> > > > index 1169237905..75a417d65c 100644
> > > > --- a/target/arm/kvm64.c
> > > > +++ b/target/arm/kvm64.c
> > > > @@ -398,7 +398,7 @@ static CPUWatchpoint *find_hw_watchpoint(CPUState *cpu, target_ulong addr)
> > > >      return NULL;
> > > >  }
> > > >
> > > > -static bool kvm_arm_pmu_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> > > > +static bool kvm_arm_dev_set_attr(CPUState *cs, struct kvm_device_attr *attr)
> > > >  {
> > > >      int err;
> > > >
> > > > @@ -427,7 +427,7 @@ void kvm_arm_pmu_init(CPUState *cs)
> > > >      if (!ARM_CPU(cs)->has_pmu) {
> > > >          return;
> > > >      }
> > > > -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> > > > +    if (!kvm_arm_dev_set_attr(cs, &attr)) {
> > > >          error_report("failed to init PMU");
> > > >          abort();
> > > >      }
> > > > @@ -444,7 +444,7 @@ void kvm_arm_pmu_set_irq(CPUState *cs, int irq)
> > > >      if (!ARM_CPU(cs)->has_pmu) {
> > > >          return;
> > > >      }
> > > > -    if (!kvm_arm_pmu_set_attr(cs, &attr)) {
> > > > +    if (!kvm_arm_dev_set_attr(cs, &attr)) {
> > > >          error_report("failed to set irq for PMU");
> > > >          abort();
> > > >      }
> > > >
> > >
> >
>

Patchew 2.1-devel-b67e4c2

© 2016 - 2026 Red Hat, Inc.