[PATCH v7 19/19] KVM: ARM64: Add a new kvm ARM PMU device

[Christoffer Dall]

On Wed, Dec 16, 2015 at 04:06:49PM +0800, Shannon Zhao wrote:
> Hi,
> 
> On 2015/12/16 15:31, Shannon Zhao wrote:
> >>>> >> > But in this case, you're returning an error if it is *not* initialized.
> >>>> >> > I understand that in that case you cannot return an interrupt number (-1
> >>>> >> > would be weird), but returning -EBUSY feels even more weird.
> >>>> >> > 
> >>>> >> > I'd settle for -ENOXIO, or something similar. Anyone having a better idea?
> >>>> >> > 
> >> > ENXIO or ENODEV would be my choice too, and add that to the
> >> > Documentation clearly describing when this error code is used.
> >> > 
> >> > By the way, why do you loop over all VCPUS to set the same value when
> >> > you can't do anything per VCPU anyway?  It seems to me it's either a
> >> > per-VM property (that you can store on the VM data structure) or it's a
> >> > true per-VCPU property?
> > This is a per-VCPU property. PMU interrupt could be PPI or SPI. For PPI
> > the interrupt numbers are same for each vcpu, while for SPI they are
> > different, so it needs to set them separately. I planned to support both
> > PPI and SPI. I think I should add support for SPI at this moment and let
> > users (QEMU) to set these interrupts for each one.
> 
> How about below vPMU Documentation?
> 
> ARM Virtual Performance Monitor Unit (vPMU)
> ===========================================
> 
> Device types supported:
>   KVM_DEV_TYPE_ARM_PMU_V3         ARM Performance Monitor Unit v3
> 
> Instantiate one PMU instance for per VCPU through this API.
> 
> Groups:
>   KVM_DEV_ARM_PMU_GRP_IRQ
>   Attributes:
>     The attr field of kvm_device_attr encodes two values:
>     bits:     | 63 .... 32 | 31 .... 0 |
>     values:   | vcpu_index |  irq_num  |
>     The irq_num describes 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.

some formatting snafus that I expect come from pasting the text in an
e-mail client.

> 
>   Errors:
>     -ENXIO: Getting or setting this attribute is not yet supported

'not yet supported' as in something we'll implement later, or as in you
need to call this other function before you can access this state?

>     -ENODEV: Getting the PMU overflow interrupt number while it's not set
>     -EBUSY: The PMU overflow interrupt is already set
>     -EINVAL: Invalid vcpu_index or irq_num supplied
> 
> 
Otherwise looks good.

Thanks,
-Christoffer