On 29/01/2015 22:48, Radim Krčmář wrote:
> In mixed modes, we musn't deliver xAPIC IPIs like x2APIC and vice versa.
> Instead of preserving the information in apic_send_ipi(), we regain it
> by converting all destinations into correct APIC MDA in the slow path.
> This allows easier reasoning about subsequent matching.
>
> kvm_apic_broadcast() had an interesting design decision:
> it didn't consider IOxAPIC 0xff as broadcast in x2APIC mode ...
> everything worked because IOxAPIC can't set that in physical mode and
> logical mode considered it as a message for first 8 VCPUs.
> This patch interprets IOxAPIC 0xff as x2APIC broadcast.
>
> Signed-off-by: Radim Krčmář <rkrcmar@xxxxxxxxxx>
> ---
> arch/x86/kvm/lapic.c | 40 +++++++++++++++++++++++++++++++++-------
> 1 file changed, 33 insertions(+), 7 deletions(-)
>
> diff --git a/arch/x86/kvm/lapic.c b/arch/x86/kvm/lapic.c
> index aae043f38548..871ce6a2485b 100644
> --- a/arch/x86/kvm/lapic.c
> +++ b/arch/x86/kvm/lapic.c
> @@ -581,15 +581,24 @@ static void apic_set_tpr(struct kvm_lapic *apic, u32 tpr)
> apic_update_ppr(apic);
> }
>
> -static bool kvm_apic_broadcast(struct kvm_lapic *apic, u32 dest)
> +static bool kvm_apic_broadcast(struct kvm_lapic *apic, u32 mda)
> {
> - return dest == (apic_x2apic_mode(apic) ?
> - X2APIC_BROADCAST : APIC_BROADCAST);
> + if (apic_x2apic_mode(apic))
> + return mda == X2APIC_BROADCAST;
> +
> + /* XXX: verify that xAPIC accepts x2APIC broadcast */
> + return GET_APIC_DEST_FIELD(mda) == APIC_BROADCAST;
> }
>
> -static bool kvm_apic_match_physical_addr(struct kvm_lapic *apic, u32 dest)
> +static bool kvm_apic_match_physical_addr(struct kvm_lapic *apic, u32 mda)
> {
> - return kvm_apic_id(apic) == dest || kvm_apic_broadcast(apic, dest);
> + if (kvm_apic_broadcast(apic, mda))
> + return true;
> +
> + if (apic_x2apic_mode(apic))
> + return mda == kvm_apic_id(apic);
> +
> + return mda == SET_APIC_DEST_FIELD(kvm_apic_id(apic));
> }
>
> static bool kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda)
> @@ -606,6 +615,7 @@ static bool kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda)
> && (logical_id & mda & 0xffff);
>
> logical_id = GET_APIC_LOGICAL_ID(logical_id);
> + mda = GET_APIC_DEST_FIELD(mda);
>
> switch (kvm_apic_get_reg(apic, APIC_DFR)) {
> case APIC_DFR_FLAT:
> @@ -620,10 +630,26 @@ static bool kvm_apic_match_logical_addr(struct kvm_lapic *apic, u32 mda)
> }
> }
>
> +/* KVM APIC implementation has two quirks
> + * - dest always begins at 0 while xAPIC MDA has offset 24,
> + * - IOxAPIC messages have to be delivered (directly) to x2APIC.
> + */
> +static u32
> +kvm_apic_mda(unsigned int dest, struct kvm_lapic *ipi, bool x2apic_dest)
Please pass two struct kvm_lapic, so that you can write
bool ipi = source != NULL;
bool x2apic_mda = apic_x2apic_mode(ipi ? source : dest);
Looks a little nicer to me at least.
> +{
> + bool x2apic_mda = ipi ? apic_x2apic_mode(ipi) : x2apic_dest;
> +
> + if (!ipi && dest == APIC_BROADCAST)
> + dest = X2APIC_BROADCAST;
This works, but it is not super-clear that you are shifting left by 24
here, and right in kvm_apic_broadcast(). What if you just make it
if (!ipi && dest == APIC_BROADCAST && x2apic_mda)
return X2APIC_BROADCAST.
?
Paolo
> +
> + return x2apic_mda ? dest : SET_APIC_DEST_FIELD(dest);
> +}
> +
> bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
> int short_hand, unsigned int dest, int dest_mode)
> {
> struct kvm_lapic *target = vcpu->arch.apic;
> + u32 mda = kvm_apic_mda(dest, source, apic_x2apic_mode(target));
>
> apic_debug("target %p, source %p, dest 0x%x, "
> "dest_mode 0x%x, short_hand 0x%x\n",
> @@ -633,9 +659,9 @@ bool kvm_apic_match_dest(struct kvm_vcpu *vcpu, struct kvm_lapic *source,
> switch (short_hand) {
> case APIC_DEST_NOSHORT:
> if (dest_mode == APIC_DEST_PHYSICAL)
> - return kvm_apic_match_physical_addr(target, dest);
> + return kvm_apic_match_physical_addr(target, mda);
> else
> - return kvm_apic_match_logical_addr(target, dest);
> + return kvm_apic_match_logical_addr(target, mda);
> case APIC_DEST_SELF:
> return target == source;
> case APIC_DEST_ALLINC:
>
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
