Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]
Groups > linux.kernel > #1351743
| From | Paolo Bonzini <pbonzini@redhat.com> |
|---|---|
| Newsgroups | linux.kernel |
| Subject | Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC |
| Date | 2016-03-07 17:00 +0100 |
| Message-ID | <ra5JF-4Li-15@gated-at.bofh.it> (permalink) |
| References | <r94PD-3Vt-3@gated-at.bofh.it> <r94Zl-40K-23@gated-at.bofh.it> |
| Organization | linux.* mail to news gateway |
On 04/03/2016 21:46, Suravee Suthikulpanit wrote:
> +#define SVM_EXIT_AVIC_INCMP_IPI 0x401
> +#define SVM_EXIT_AVIC_NOACCEL 0x402
>
> +enum avic_incmp_ipi_err_code {
> + AVIC_INCMP_IPI_ERR_INVALID_INT_TYPE,
> + AVIC_INCMP_IPI_ERR_TARGET_NOT_RUN,
> + AVIC_INCMP_IPI_ERR_INV_TARGET,
> + AVIC_INCMP_IPI_ERR_INV_BK_PAGE,
> +};
> +
Please do not abbreviate names and use the same definition as the
manual; these are "IPI delivery failure causes", which we can shorten to
"IPI failure causes". Putting it together gives:
#define SVM_EXIT_AVIC_INCOMPLETE_IPI 0x401
#define SVM_EXIT_AVIC_UNACCELERATED_ACCESS 0x402
enum avic_ipi_failure_cause {
AVIC_IPI_FAILURE_INVALID_INT_TYPE,
AVIC_IPI_FAILURE_TARGET_NOT_RUNNING,
AVIC_IPI_FAILURE_INVALID_TARGET,
AVIC_IPI_FAILURE_INVALID_BACKING_PAGE,
};
Likewise, do not abbreviate function names.
> +#define APIC_SHORT_MASK 0xc0000
> +#define APIC_DEST_MASK 0x800
Please move these to lapic.h too in patch 1.
> + case AVIC_INCMP_IPI_ERR_INV_TARGET:
> + pr_err("%s: Invalid IPI target (icr=%#08x:%08x, idx=%u)\n",
> + __func__, icrh, icrl, index);
> + BUG();
> + break;
> + case AVIC_INCMP_IPI_ERR_INV_BK_PAGE:
> + pr_err("%s: Invalid bk page (icr=%#08x:%08x, idx=%u)\n",
> + __func__, icrh, icrl, index);
> + BUG();
> + break;
Please use WARN(1, "%s: Invalid bk page (icr=%#08x:%08x, idx=%u)\n",
__func__, icrh, icrl, index) (and likewise for invalid target) instead
of BUG().
>
> + pr_debug("%s: offset=%#x, val=%#x, (cpu=%x) (vcpu_id=%x)\n",
> + __func__, offset, reg, svm->vcpu.cpu, svm->vcpu.vcpu_id);
> +
> + switch (offset) {
> + case APIC_ID: {
> + u32 aid = (reg >> 24) & 0xff;
> + struct svm_avic_phy_ait_entry *o_ent =
> + avic_get_phy_ait_entry(&svm->vcpu, svm->vcpu.vcpu_id);
> + struct svm_avic_phy_ait_entry *n_ent =
> + avic_get_phy_ait_entry(&svm->vcpu, aid);
> +
> + if (!n_ent || !o_ent)
> + return 0;
> +
> + pr_debug("%s: APIC_ID=%#x (id=%x)\n", __func__, reg, aid);
> +
> + /* We need to move phy_apic_entry to new offset */
> + *n_ent = *o_ent;
> + *((u64 *)o_ent) = 0ULL;
> + break;
> + }
> + case APIC_LDR: {
> + int ret, lid;
> + int dlid = (reg >> 24) & 0xff;
> +
> + if (!dlid)
> + return 0;
> +
> + lid = ffs(dlid) - 1;
> + pr_debug("%s: LDR=%0#10x (lid=%x)\n", __func__, reg, lid);
> + ret = avic_init_log_apic_entry(&svm->vcpu, svm->vcpu.vcpu_id,
> + lid);
> + if (ret)
> + return 0;
> +
> + break;
> + }
> + case APIC_DFR: {
> + u32 mod = (*avic_get_bk_page_entry(svm, offset) >> 28) & 0xf;
> +
> + pr_debug("%s: DFR=%#x (%s)\n", __func__,
> + mod, (mod == 0xf) ? "flat" : "cluster");
> +
> + /*
> + * We assume that all local APICs are using the same type.
> + * If this changes, we need to rebuild the AVIC logical
> + * APID id table with subsequent write to APIC_LDR.
> + */
> + if (vm_data->ldr_mode != mod) {
> + clear_page(page_address(vm_data->avic_log_ait_page));
> + vm_data->ldr_mode = mod;
> + }
> + break;
> + }
> + case APIC_TMICT: {
> + u32 val = kvm_apic_get_reg(apic, APIC_TMICT);
> +
> + pr_debug("%s: TMICT=%#x,%#x\n", __func__, val, reg);
> + break;
> + }
> + case APIC_ESR: {
> + u32 val = kvm_apic_get_reg(apic, APIC_ESR);
> +
> + pr_debug("%s: ESR=%#x,%#x\n", __func__, val, reg);
> + break;
> + }
> + case APIC_LVTERR: {
> + u32 val = kvm_apic_get_reg(apic, APIC_LVTERR);
> +
> + pr_debug("%s: LVTERR=%#x,%#x\n", __func__, val, reg);
> + break;
> + }
> + default:
> + break;
Please use a single tracepoint instead of all these pr_debug statements.
The tracepoint can convert APIC register offsets to APIC register
names, like the existing kvm_apic tracepoint. Also please remove the
TMICT/ESR/LVTERR cases, since they do nothing but debugging.
Notice how a single tracepoint can be used for multiple functions, the
example being kvm_avic as well.
Existing tracepoints in fact make most of the pr_debug statements
unnecessary.
Paolo
Back to linux.kernel | Previous | Next — Previous in thread | Next in thread | Find similar | Unroll thread
[PART1 RFC v2 00/10] KVM: x86: Introduce SVM AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-04 21:50 +0100
[PART1 RFC v2 03/10] svm: Introduce new AVIC VMCB registers Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-04 21:50 +0100
Re: [PART1 RFC v2 03/10] svm: Introduce new AVIC VMCB registers Paolo Bonzini <pbonzini@redhat.com> - 2016-03-07 16:50 +0100
Re: [PART1 RFC v2 03/10] svm: Introduce new AVIC VMCB registers Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-14 08:50 +0100
Re: [PART1 RFC v2 03/10] svm: Introduce new AVIC VMCB registers Paolo Bonzini <pbonzini@redhat.com> - 2016-03-14 13:30 +0100
Re: [PART1 RFC v2 03/10] svm: Introduce new AVIC VMCB registers Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-15 14:00 +0100
[PART1 RFC v2 08/10] svm: Do not expose x2APIC when enable AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-04 21:50 +0100
[PART1 RFC v2 05/10] KVM: x86: Detect and Initialize AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-04 21:50 +0100
Re: [PART1 RFC v2 05/10] KVM: x86: Detect and Initialize AVIC support Paolo Bonzini <pbonzini@redhat.com> - 2016-03-07 17:50 +0100
Re: [PART1 RFC v2 05/10] KVM: x86: Detect and Initialize AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-15 18:20 +0100
Re: [PART1 RFC v2 05/10] KVM: x86: Detect and Initialize AVIC support Paolo Bonzini <pbonzini@redhat.com> - 2016-03-15 18:30 +0100
Re: [PART1 RFC v2 05/10] KVM: x86: Detect and Initialize AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-16 07:30 +0100
Re: [PART1 RFC v2 05/10] KVM: x86: Detect and Initialize AVIC support Paolo Bonzini <pbonzini@redhat.com> - 2016-03-16 08:30 +0100
Re: [PART1 RFC v2 05/10] KVM: x86: Detect and Initialize AVIC support Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-16 09:30 +0100
Re: [PART1 RFC v2 05/10] KVM: x86: Detect and Initialize AVIC support Paolo Bonzini <pbonzini@redhat.com> - 2016-03-16 12:20 +0100
[PART1 RFC v2 01/10] KVM: x86: Misc LAPIC changes to exposes helper functions Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-04 22:00 +0100
[PART1 RFC v2 04/10] svm: clean up V_TPR, V_IRQ, V_INTR_PRIO, and V_INTR_MASKING Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-04 22:00 +0100
[PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-04 22:00 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-03-07 17:00 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-03-08 23:10 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-03-09 12:00 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-03-09 22:00 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-03-10 20:40 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-03-10 21:00 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Radim Krčmář <rkrcmar@redhat.com> - 2016-03-10 21:50 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Suravee Suthikulpanit <Suravee.Suthikulpanit@amd.com> - 2016-03-17 05:00 +0100
Re: [PART1 RFC v2 07/10] svm: Add VMEXIT handlers for AVIC Paolo Bonzini <pbonzini@redhat.com> - 2016-03-17 10:40 +0100
csiph-web