Re: [Qemu-devel] [PATCH RFC 0/4] vGICv3 support

[Christoffer Dall]

On Fri, May 22, 2015 at 01:58:40PM +0300, Pavel Fedin wrote:
>  This is my alternative to Ashok's vGICv3 patch
> (https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg03021.html), which
> i am currently working on. It addresses vGIC capability verification issue
> (kvm_irqchip_create() / kvm_arch_irqchip_create()), as well as offers better
> code structure (v3 code separated from v2).
>  This patchset applies on top of this:
> https://lists.gnu.org/archive/html/qemu-devel/2015-05/msg00943.html. Note that
> GIC type selection still relies on machine name (virt-v3 vs virt), and not on
> machine option. Since libvirt has recently introduced support for extra 
> options,
> i have absolutely nothing against Ashok's approach. I just did not change this
> yet because it would affect my testing environment. The aim of this RFC is to
> focus on vGICv3 implementation and related changes. And yes, i agree that v2 
> and
> v3 now have some copypasted code, and this is TBD.

This cover letter is not really helpful as it only describes the history
and circumstances of how this patch came to be.

It would be helpful if the beginning of this cover letter focuses on
what the patch series does and which design decisions have been taken to
shape the patches the way they are.

I don't understand the whole background thing about libvirt and I don't
at all understand the thing about copy-pasted code...??

A generally good approach to writing a cover letter is to follow this
skeleton:

---
This series implements...<what>

We accomplish this by...<design decisions>

[Optional] Patches 1-3 <do something preliminary>, patches 4-8 <do something 
else>...

[Optional] Note <something special, possibly historical>

Changes since vX:
---

I think it would be good if you could re-spin this series based on
Eric's comments on the code, my comments on the patch style, and Peter's
advise on using machine properties for GICv3.

Do you have cycles to continue working on this?

-Christoffer

> 
> Pavel Fedin (4):
>   Add virt-v3 machine that uses GIC-500
>   Set kernel_irqchip_type for other ARM boards which use GIC
>   First bits of vGICv3 support:
>   Initial implementation of vGICv3.
> 
>  hw/arm/exynos4_boards.c |   1 +
>  hw/arm/realview.c       |   1 +
>  hw/arm/vexpress.c       |   1 +
>  hw/arm/virt.c           | 148 ++++++++++++++++++++-----
>  hw/intc/Makefile.objs   |   1 +
>  hw/intc/arm_gicv3_kvm.c | 283 
> ++++++++++++++++++++++++++++++++++++++++++++++++
>  include/hw/boards.h     |   1 +
>  include/sysemu/kvm.h    |   3 +-
>  kvm-all.c               |   2 +-
>  stubs/kvm.c             |   2 +-
>  target-arm/kvm.c        |   8 +-
>  11 files changed, 419 insertions(+), 32 deletions(-)
>  create mode 100644 hw/intc/arm_gicv3_kvm.c
> 
> -- 
> 1.9.5.msysgit.0
> 
>