On 23/11/2023 2:03 pm, Roger Pau Monné wrote:
> On Thu, Nov 23, 2023 at 12:21:36PM +0000, Alejandro Vallejo wrote:
>> On Thu, Nov 23, 2023 at 10:03:12AM +0100, Roger Pau Monné wrote:
>>> On Wed, Nov 22, 2023 at 04:08:17PM +0000, Alejandro Vallejo wrote:
>>>> + if ( vlapic_domain(vlapic)->arch.hvm.x2apic_ldr_bug_with_vcpu_id )
>>>> + apic_ldr = x2apic_ldr_from_id(vcpu_id);
>>>>
>>>> - vlapic_set_reg(vlapic, APIC_ID, id * 2);
>>>> - vlapic_set_reg(vlapic, APIC_LDR, ldr);
>>>> + vlapic_set_reg(vlapic, APIC_ID, apic_id);
>>>> + vlapic_set_reg(vlapic, APIC_LDR, apic_ldr);
>>>> }
>>>>
>>>> int guest_wrmsr_apic_base(struct vcpu *v, uint64_t val)
>>>> @@ -1498,27 +1508,35 @@ static int cf_check lapic_save_regs(struct vcpu
>>>> *v, hvm_domain_context_t *h)
>>>> */
>>>> static void lapic_load_fixup(struct vlapic *vlapic)
>>>> {
>>>> - uint32_t id = vlapic->loaded.id;
>>>> + /* Skip fixups on xAPIC mode, or if the x2APIC LDR is already correct
>>>> */
>>>> + if ( !vlapic_x2apic_mode(vlapic) ||
>>>> + (vlapic->loaded.ldr == x2apic_ldr_from_id(vlapic->loaded.id)) )
>>>> + return;
>>>>
>>>> - if ( vlapic_x2apic_mode(vlapic) && id && vlapic->loaded.ldr == 1 )
>>>> - {
>>>> + if ( vlapic->loaded.ldr == 1 )
>>>> + /*
>>>> + * Xen <= 4.4 had a bug by which all the APICs configured in x2APIC
>>>> + * mode got LDR = 1. We can't leave it as-is because it assigned
>>>> the
>>>> + * same LDR to every CPU. We'll fix fix the bug now and assign an
>>>> + * LDR value consistent with the APIC ID.
>>>> + */
>>>> + set_x2apic_id(vlapic);
>>>> + else if ( vlapic->loaded.ldr ==
>>>> + x2apic_ldr_from_id(vlapic_vcpu(vlapic)->vcpu_id) )
>>>> /*
>>>> - * This is optional: ID != 0 contradicts LDR == 1. It's being
>>>> added
>>>> - * to aid in eventual debugging of issues arising from the fixup
>>>> done
>>>> - * here, but can be dropped as soon as it is found to conflict
>>>> with
>>>> - * other (future) changes.
>>>> + * This is a migration from a broken Xen between 4.4 and 4.18 and
>>>> + * we must _PRESERVE_ LDRs so new vCPUs use consistent
>>>> derivations.
>>> Not sure if we should try to avoid mentioning specific versions in the
>>> comments, as I this fix will be backported to stable branches (I hope),
>>> and hence those will no longer be affected.
>> Hence the "broken Xen" part of the paragraphs. Not every 4.18 will have the
>> problem, but it shouldn't be seen in 4.19 onwards. I think there's value in
>> stating the versions that "may" exhibit problems, but this is all
>> subjective
> FE.
This patch has a high chance of being backported. Old version numbers
(4.4 in this case) are critical information.
For the new end, "to date (Nov 2023)" or similar tends to works better,
because it also doesn't go stale when backported.
If a version is necessary/preferred, then "4.19 dev window" also works well.
~Andrew
