[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [PATCH v4] xen/x86: On x2APIC mode, derive LDR from APIC ID
On 24/11/2023 22:05, Andrew Cooper wrote: diff --git a/xen/arch/x86/hvm/vlapic.c b/xen/arch/x86/hvm/vlapic.c index 5cb87f8649..cd4701c5a2 100644 --- a/xen/arch/x86/hvm/vlapic.c +++ b/xen/arch/x86/hvm/vlapic.c @@ -1061,13 +1061,26 @@ static const struct hvm_mmio_ops vlapic_mmio_ops = { .write = vlapic_mmio_write, };+static uint32_t x2apic_ldr_from_id(uint32_t id)+{ + return ((id & ~0xf) << 12) | (1 << (id & 0xf)); +} + static void set_x2apic_id(struct vlapic *vlapic) {const struct vcpu *v = vlapic_vcpu(vlapic); seeing as you've got the expression used twice already. With that, the following logic is shorter too; you can get away with dropping the vcpu_id variable as v->vcpu_id is the more common form to use in Xen. Twice? I can see a vague raison-d'etre in lapic_load_fixup(), but there's a single occurence here. We must preserve LDRs so new vCPUs use consistent + * derivations and existing guests, which may have already read the + * LDR at the source host, aren't surprised when interrupts stop + * working the way they did at the other end. */ - if ( GET_xAPIC_ID(id) != vlapic_vcpu(vlapic)->vcpu_id * 2 || - id != SET_xAPIC_ID(GET_xAPIC_ID(id)) ) - printk(XENLOG_G_WARNING "%pv: bogus APIC ID %#x loaded\n", - vlapic_vcpu(vlapic), id); - set_x2apic_id(vlapic); - } - else /* Undo an eventual earlier fixup. */ - { - vlapic_set_reg(vlapic, APIC_ID, id); - vlapic_set_reg(vlapic, APIC_LDR, vlapic->loaded.ldr); - } + vlapic_domain(vlapic)->arch.hvm.bug_x2apic_ldr_vcpu_id = true; + else + printk(XENLOG_G_WARNING + "%pv: bogus x2APIC loaded id=%#x ldr=%#x (expected ldr=%#x)\n",%pv: bogus loaded x2APIC ID %#x, LDR %#x, expected LDR %#x\n If you properly capitalise x2APIC, you should capitalise ID and LDR. The other changes are matter of taste, but make for a less cluttered log message IMO. "bogus x2APIC loaded" is meant to be a sentence followed by key-value pairs. Uppercasing the keys is fine (albeit unnecessary, IMO), but you choice of word order feels backwards. This is a long list of minor niggles, but they're all style/comment issues, and nothing to do with the logic itself. I'm happy to fix them all up on commit, and here is the result with them merged: https://xenbits.xen.org/gitweb/?p=people/andrewcoop/xen.git;a=commitdiff;h=953dcb0317d20959ffee14e404595cfbb66c607a for you to glance over. ~Andrew Except for the 2 bits pointed out, the others are cosmetic changesI don't particularly mind. Jan spotted a typo in the second comment block in lapic_load_fixup() that should be corrected too. (s/fix fix/fix/) Cheers, Alejandro
|
Lists.xenproject.org is hosted with RackSpace, monitoring our |