diff mbox series

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

Message ID 3cc31df5191ae6b03e060ccd8e82df74416a3ef5.1596768588.git.haibo.xu@linaro.org
State New
Headers show
Series target/arm: Add vSPE support to KVM guest | expand

Commit Message

Haibo Xu Aug. 7, 2020, 8:10 a.m. UTC 
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(-)

-- 
2.17.1

Comments

Philippe Mathieu-Daudé Aug. 7, 2020, 8:19 a.m. UTC | #1 
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();

>      }

>
Haibo Xu Aug. 10, 2020, 2:48 a.m. UTC | #2 
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();

> >      }

> >

>
Andrew Jones Aug. 10, 2020, 10:29 a.m. UTC | #3 
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();

> > >      }

> > >

> >

>
Haibo Xu Aug. 11, 2020, 1:13 a.m. UTC | #4 
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();

> > > >      }

> > > >

> > >

> >

>
diff mbox series

Patch

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();
     }