All of lore.kernel.org
 help / color / mirror / Atom feed
From: Andrew Jones <drjones@redhat.com>
To: Peter Maydell <peter.maydell@linaro.org>
Cc: Shannon Zhao <zhaoshenglong@huawei.com>,
	"kvmarm@lists.cs.columbia.edu" <kvmarm@lists.cs.columbia.edu>,
	Marc Zyngier <marc.zyngier@arm.com>,
	Christoffer Dall <christoffer.dall@linaro.org>,
	kvm-devel <kvm@vger.kernel.org>,
	Will Deacon <will.deacon@arm.com>,
	Shannon Zhao <shannon.zhao@linaro.org>,
	arm-mail-list <linux-arm-kernel@lists.infradead.org>
Subject: Re: [PATCH v8 20/20] KVM: ARM64: Add a new kvm ARM PMU device
Date: Thu, 7 Jan 2016 21:36:47 +0100	[thread overview]
Message-ID: <20160107203647.GJ6199@hawk.localdomain> (raw)
In-Reply-To: <CAFEAcA-Qcda7TSFLSeykLHqZYdP6tYdEjPe2uHN6f_+omqWFZg@mail.gmail.com>

On Thu, Jan 07, 2016 at 02:56:15PM +0000, Peter Maydell wrote:
> On 7 January 2016 at 14:49, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> >>> +
> >>> +Groups:
> >>> +  KVM_DEV_ARM_PMU_GRP_IRQ
> >>> +  Attributes:
> >>> +    The attr field of kvm_device_attr encodes one value:
> >>> +    bits:     | 63 .... 32 | 31 ....  0 |
> >>> +    values:   |  reserved  | vcpu_index |
> >>> +    A value describing the PMU overflow interrupt number for the specified
> >>> +    vcpu_index vcpu. This interrupt could be a PPI or SPI, but for one VM the
> >>> +    interrupt type must be same for each vcpu. As a PPI, the interrupt number is
> >>> +    same for all vcpus, while as a SPI it must be different for each vcpu.
> >>
> >> I see we're using vcpu_index rather than MPIDR affinity value
> >> for specifying which CPU we're configuring. Is this in line with
> >> our planned API for GICv3 configuration?
> >>
> > Here vcpu_index is used to indexing the vCPU, no special use.
> 
> Yes, but you can identify the CPU by index, or by its MPIDR.
> We had a discussion about which was the best way for doing
> the VGIC API, and I can't remember which way round we ended up
> going for. Whichever we chose, we should do the same thing here.

I think we should start up a new discussion on this. My understanding,
after a chat with Igor, who was involved in the untangling of vcpu-id and
apic-id for x86, is that using vcpu-id is preferred, unless of course
the device expects an apic-id/mpidr, in which case there's no reason to
translate it on both sides.

Other things to discuss are
1) Consistent use of the term vcpu-id vs. vcpu-index in the documentation.
   I just brought that up in my last reply to this patch.
2) Extending a vcpu-index [id] beyond 8-bits.
3) Should vcpu-ids be reusable after hotunplugging them?
4) Devices like this pmu and the vgic have some global (all vcpus wide)
   properties, but then also vcpu specific registers, which require the
   vcpu-index parameter. Should we instead create a "vcpu device ioctl"
   for these? I.e. extend SET/GET_DEVICE_ATTR to also be of type
   'vcpu ioctl', allowing the vcpu-index parameter to be dropped?

Thanks,
drew

WARNING: multiple messages have this Message-ID (diff)
From: drjones@redhat.com (Andrew Jones)
To: linux-arm-kernel@lists.infradead.org
Subject: [PATCH v8 20/20] KVM: ARM64: Add a new kvm ARM PMU device
Date: Thu, 7 Jan 2016 21:36:47 +0100	[thread overview]
Message-ID: <20160107203647.GJ6199@hawk.localdomain> (raw)
In-Reply-To: <CAFEAcA-Qcda7TSFLSeykLHqZYdP6tYdEjPe2uHN6f_+omqWFZg@mail.gmail.com>

On Thu, Jan 07, 2016 at 02:56:15PM +0000, Peter Maydell wrote:
> On 7 January 2016 at 14:49, Shannon Zhao <zhaoshenglong@huawei.com> wrote:
> >>> +
> >>> +Groups:
> >>> +  KVM_DEV_ARM_PMU_GRP_IRQ
> >>> +  Attributes:
> >>> +    The attr field of kvm_device_attr encodes one value:
> >>> +    bits:     | 63 .... 32 | 31 ....  0 |
> >>> +    values:   |  reserved  | vcpu_index |
> >>> +    A value describing the PMU overflow interrupt number for the specified
> >>> +    vcpu_index vcpu. This interrupt could be a PPI or SPI, but for one VM the
> >>> +    interrupt type must be same for each vcpu. As a PPI, the interrupt number is
> >>> +    same for all vcpus, while as a SPI it must be different for each vcpu.
> >>
> >> I see we're using vcpu_index rather than MPIDR affinity value
> >> for specifying which CPU we're configuring. Is this in line with
> >> our planned API for GICv3 configuration?
> >>
> > Here vcpu_index is used to indexing the vCPU, no special use.
> 
> Yes, but you can identify the CPU by index, or by its MPIDR.
> We had a discussion about which was the best way for doing
> the VGIC API, and I can't remember which way round we ended up
> going for. Whichever we chose, we should do the same thing here.

I think we should start up a new discussion on this. My understanding,
after a chat with Igor, who was involved in the untangling of vcpu-id and
apic-id for x86, is that using vcpu-id is preferred, unless of course
the device expects an apic-id/mpidr, in which case there's no reason to
translate it on both sides.

Other things to discuss are
1) Consistent use of the term vcpu-id vs. vcpu-index in the documentation.
   I just brought that up in my last reply to this patch.
2) Extending a vcpu-index [id] beyond 8-bits.
3) Should vcpu-ids be reusable after hotunplugging them?
4) Devices like this pmu and the vgic have some global (all vcpus wide)
   properties, but then also vcpu specific registers, which require the
   vcpu-index parameter. Should we instead create a "vcpu device ioctl"
   for these? I.e. extend SET/GET_DEVICE_ATTR to also be of type
   'vcpu ioctl', allowing the vcpu-index parameter to be dropped?

Thanks,
drew

  reply	other threads:[~2016-01-07 20:36 UTC|newest]

Thread overview: 197+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-12-22  8:07 [PATCH v8 00/20] KVM: ARM64: Add guest PMU support Shannon Zhao
2015-12-22  8:07 ` Shannon Zhao
2015-12-22  8:07 ` Shannon Zhao
2015-12-22  8:07 ` [PATCH v8 01/20] ARM64: Move PMU register related defines to asm/pmu.h Shannon Zhao
2015-12-22  8:07   ` Shannon Zhao
2015-12-22  8:07   ` Shannon Zhao
2016-01-07 10:20   ` Marc Zyngier
2016-01-07 10:20     ` Marc Zyngier
2015-12-22  8:07 ` [PATCH v8 02/20] KVM: ARM64: Define PMU data structure for each vcpu Shannon Zhao
2015-12-22  8:07   ` Shannon Zhao
2015-12-22  8:07   ` Shannon Zhao
2016-01-07 10:21   ` Marc Zyngier
2016-01-07 10:21     ` Marc Zyngier
2016-01-07 19:07   ` Andrew Jones
2016-01-07 19:07     ` Andrew Jones
2015-12-22  8:07 ` [PATCH v8 03/20] KVM: ARM64: Add offset defines for PMU registers Shannon Zhao
2015-12-22  8:07   ` Shannon Zhao
2015-12-22  8:07   ` Shannon Zhao
2016-01-07 10:23   ` Marc Zyngier
2016-01-07 10:23     ` Marc Zyngier
2015-12-22  8:07 ` [PATCH v8 04/20] KVM: ARM64: Add access handler for PMCR register Shannon Zhao
2015-12-22  8:07   ` Shannon Zhao
2015-12-22  8:07   ` Shannon Zhao
2016-01-07 10:43   ` Marc Zyngier
2016-01-07 10:43     ` Marc Zyngier
2016-01-07 11:16     ` Shannon Zhao
2016-01-07 11:16       ` Shannon Zhao
2016-01-07 11:16       ` Shannon Zhao
2015-12-22  8:08 ` [PATCH v8 05/20] KVM: ARM64: Add access handler for PMSELR register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 10:43   ` Marc Zyngier
2016-01-07 10:43     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 06/20] KVM: ARM64: Add access handler for PMCEID0 and PMCEID1 register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 10:44   ` Marc Zyngier
2016-01-07 10:44     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 07/20] KVM: ARM64: PMU: Add perf event map and introduce perf event creating function Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 10:55   ` Marc Zyngier
2016-01-07 10:55     ` Marc Zyngier
2016-01-07 13:48   ` Marc Zyngier
2016-01-07 13:48     ` Marc Zyngier
2016-01-07 14:00     ` Shannon Zhao
2016-01-07 14:00       ` Shannon Zhao
2016-01-07 14:00       ` Shannon Zhao
2015-12-22  8:08 ` [PATCH v8 08/20] KVM: ARM64: Add access handler for event typer register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 11:03   ` Marc Zyngier
2016-01-07 11:03     ` Marc Zyngier
2016-01-07 11:11     ` Shannon Zhao
2016-01-07 11:11       ` Shannon Zhao
2016-01-07 11:11       ` Shannon Zhao
2016-01-07 12:36     ` Shannon Zhao
2016-01-07 12:36       ` Shannon Zhao
2016-01-07 12:36       ` Shannon Zhao
2016-01-07 13:15       ` Marc Zyngier
2016-01-07 13:15         ` Marc Zyngier
2016-01-07 12:09   ` Shannon Zhao
2016-01-07 12:09     ` Shannon Zhao
2016-01-07 12:09     ` Shannon Zhao
2016-01-07 13:01     ` Marc Zyngier
2016-01-07 13:01       ` Marc Zyngier
2016-01-07 19:17   ` Andrew Jones
2016-01-07 19:17     ` Andrew Jones
2015-12-22  8:08 ` [PATCH v8 09/20] KVM: ARM64: Add access handler for event counter register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 11:06   ` Marc Zyngier
2016-01-07 11:06     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 10/20] KVM: ARM64: Add access handler for PMCNTENSET and PMCNTENCLR register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 11:09   ` Marc Zyngier
2016-01-07 11:09     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 11/20] KVM: ARM64: Add access handler for PMINTENSET and PMINTENCLR register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 11:13   ` Marc Zyngier
2016-01-07 11:13     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 12/20] KVM: ARM64: Add access handler for PMOVSSET and PMOVSCLR register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 11:14   ` Marc Zyngier
2016-01-07 11:14     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 13/20] KVM: ARM64: Add access handler for PMSWINC register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 11:29   ` Marc Zyngier
2016-01-07 11:29     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 14/20] KVM: ARM64: Add helper to handle PMCR register bits Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 11:59   ` Marc Zyngier
2016-01-07 11:59     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 15/20] KVM: ARM64: Add a helper to forward trap to guest EL1 Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08 ` [PATCH v8 16/20] KVM: ARM64: Add access handler for PMUSERENR register Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 10:14   ` Marc Zyngier
2016-01-07 10:14     ` Marc Zyngier
2016-01-07 11:15     ` Shannon Zhao
2016-01-07 11:15       ` Shannon Zhao
2016-01-07 11:15       ` Shannon Zhao
2015-12-22  8:08 ` [PATCH v8 17/20] KVM: ARM64: Add PMU overflow interrupt routing Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 13:28   ` Marc Zyngier
2016-01-07 13:28     ` Marc Zyngier
2016-01-07 13:28     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 18/20] KVM: ARM64: Reset PMU state when resetting vcpu Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 13:39   ` Marc Zyngier
2016-01-07 13:39     ` Marc Zyngier
2016-01-07 13:39     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 19/20] KVM: ARM64: Free perf event of PMU when destroying vcpu Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 13:51   ` Marc Zyngier
2016-01-07 13:51     ` Marc Zyngier
2015-12-22  8:08 ` [PATCH v8 20/20] KVM: ARM64: Add a new kvm ARM PMU device Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2015-12-22  8:08   ` Shannon Zhao
2016-01-07 13:56   ` Marc Zyngier
2016-01-07 13:56     ` Marc Zyngier
2016-01-07 14:35     ` Shannon Zhao
2016-01-07 14:35       ` Shannon Zhao
2016-01-07 14:35       ` Shannon Zhao
2016-01-07 14:36   ` Peter Maydell
2016-01-07 14:36     ` Peter Maydell
2016-01-07 14:49     ` Shannon Zhao
2016-01-07 14:49       ` Shannon Zhao
2016-01-07 14:56       ` Peter Maydell
2016-01-07 14:56         ` Peter Maydell
2016-01-07 20:36         ` Andrew Jones [this message]
2016-01-07 20:36           ` Andrew Jones
2016-01-09 12:29           ` Christoffer Dall
2016-01-09 12:29             ` Christoffer Dall
2016-01-09 15:03             ` Marc Zyngier
2016-01-09 15:03               ` Marc Zyngier
2016-01-11  8:45               ` Shannon Zhao
2016-01-11  8:45                 ` Shannon Zhao
2016-01-11  8:59                 ` Marc Zyngier
2016-01-11  8:59                   ` Marc Zyngier
2016-01-11 11:52                   ` Andrew Jones
2016-01-11 11:52                     ` Andrew Jones
2016-01-11 12:03                     ` Shannon Zhao
2016-01-11 12:03                       ` Shannon Zhao
2016-01-11 14:07               ` Andrew Jones
2016-01-11 14:07                 ` Andrew Jones
2016-01-11 15:09                 ` Christoffer Dall
2016-01-11 15:09                   ` Christoffer Dall
2016-01-11 16:09                   ` Andrew Jones
2016-01-11 16:09                     ` Andrew Jones
2016-01-11 16:13                     ` Peter Maydell
2016-01-11 16:13                       ` Peter Maydell
2016-01-11 16:48                       ` Andrew Jones
2016-01-11 16:48                         ` Andrew Jones
2016-01-11 16:21                     ` Andrew Jones
2016-01-11 16:21                       ` Andrew Jones
2016-01-11 16:29                       ` Peter Maydell
2016-01-11 16:29                         ` Peter Maydell
2016-01-11 16:44                         ` Andrew Jones
2016-01-11 16:44                           ` Andrew Jones
2016-01-08  3:06         ` Shannon Zhao
2016-01-08  3:06           ` Shannon Zhao
2016-01-08 10:24           ` Peter Maydell
2016-01-08 10:24             ` Peter Maydell
2016-01-08 12:15             ` Shannon Zhao
2016-01-08 12:15               ` Shannon Zhao
2016-01-08 12:56               ` Peter Maydell
2016-01-08 12:56                 ` Peter Maydell
2016-01-08 13:31                 ` Shannon Zhao
2016-01-08 13:31                   ` Shannon Zhao
2016-01-07 20:18   ` Andrew Jones
2016-01-07 20:18     ` Andrew Jones
2016-01-08  2:53     ` Shannon Zhao
2016-01-08  2:53       ` Shannon Zhao
2016-01-08  2:53       ` Shannon Zhao
2016-01-08 11:22       ` Andrew Jones
2016-01-08 11:22         ` Andrew Jones
2016-01-08 15:20         ` Andrew Jones
2016-01-08 15:20           ` Andrew Jones
2016-01-08 15:59           ` Andrew Jones
2016-01-08 15:59             ` Andrew Jones
2016-01-07 14:10 ` [PATCH v8 00/20] KVM: ARM64: Add guest PMU support Marc Zyngier
2016-01-07 14:10   ` Marc Zyngier
2016-01-07 14:12   ` Will Deacon
2016-01-07 14:12     ` Will Deacon
2016-01-07 14:21     ` Marc Zyngier
2016-01-07 14:21       ` Marc Zyngier

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20160107203647.GJ6199@hawk.localdomain \
    --to=drjones@redhat.com \
    --cc=christoffer.dall@linaro.org \
    --cc=kvm@vger.kernel.org \
    --cc=kvmarm@lists.cs.columbia.edu \
    --cc=linux-arm-kernel@lists.infradead.org \
    --cc=marc.zyngier@arm.com \
    --cc=peter.maydell@linaro.org \
    --cc=shannon.zhao@linaro.org \
    --cc=will.deacon@arm.com \
    --cc=zhaoshenglong@huawei.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.