|
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index] Re: [Xen-devel] [PATCH 3/8] x86/SVM: Add AVIC vmexit handlers
On 04/04/18 00:01, Janakarajan Natarajan wrote:
> From: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
>
> AVIC introduces two new #vmexit handlers:
>
> VMEXIT_INCOMP_IPI:
> This occurs when an IPI could not be delivered to all targeted guest
> virtual processors because at least one guest virtual processor
> was not allocated to a physical core at the time. In this case,
> Xen would try to emulate IPI.
>
> VMEXIT_DO_NOACCEL:
> This occurs when a guest access to an APIC register that cannot be
> accelerated by AVIC. In this case, Xen tries to emulate register accesses.
>
> This fault is also generated if an EOI is attempted when the highest priority
> in-service interrupt is set for level-triggered mode.
>
> This patch also declare vlapic_read_aligned() and vlapic_reg_write()
> as non-static to expose them to be used by AVIC.
>
> Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@xxxxxxx>
> Signed-off-by: Janakarajan Natarajan <Janakarajan.Natarajan@xxxxxxx>
> ---
> xen/arch/x86/hvm/svm/avic.c | 296
> +++++++++++++++++++++++++++++++++++++
> xen/arch/x86/hvm/svm/svm.c | 8 +
> xen/arch/x86/hvm/vlapic.c | 4 +-
> xen/include/asm-x86/hvm/svm/avic.h | 3 +
> xen/include/asm-x86/hvm/svm/vmcb.h | 6 +
> xen/include/asm-x86/hvm/vlapic.h | 4 +
> 6 files changed, 319 insertions(+), 2 deletions(-)
>
> diff --git a/xen/arch/x86/hvm/svm/avic.c b/xen/arch/x86/hvm/svm/avic.c
> index 8108698911..e112469774 100644
> --- a/xen/arch/x86/hvm/svm/avic.c
> +++ b/xen/arch/x86/hvm/svm/avic.c
> @@ -19,6 +19,7 @@
> #include <xen/sched.h>
> #include <xen/stdbool.h>
> #include <asm/acpi.h>
> +#include <asm/apic.h>
> #include <asm/apicdef.h>
> #include <asm/atomic.h>
> #include <asm/event.h>
> @@ -37,6 +38,8 @@
>
> #define AVIC_VAPIC_BAR_MASK (((1ULL << 40) - 1) << PAGE_SHIFT)
>
> +#define AVIC_UNACCEL_ACCESS_OFFSET_MASK 0xFF0
> +
> /*
> * Note:
> * Currently, svm-avic mode is not supported with nested virtualization.
> @@ -101,6 +104,8 @@ int svm_avic_dom_init(struct domain *d)
> d->arch.hvm_domain.svm.avic_physical_id_table_pg = pg;
> d->arch.hvm_domain.svm.avic_physical_id_table =
> __map_domain_page_global(pg);
>
> + spin_lock_init(&d->arch.hvm_domain.svm.avic_dfr_mode_lock);
> +
> return ret;
> err_out:
> svm_avic_dom_destroy(d);
> @@ -181,6 +186,297 @@ int svm_avic_init_vmcb(struct vcpu *v)
> }
>
> /*
> + * Note:
> + * This function handles the AVIC_INCOMP_IPI #vmexit when AVIC is enabled.
> + * The hardware generates this fault when an IPI could not be delivered
> + * to all targeted guest virtual processors because at least one guest
> + * virtual processor was not allocated to a physical core at the time.
> + */
> +void svm_avic_vmexit_do_incomp_ipi(struct cpu_user_regs *regs)
> +{
> + struct vcpu *curr = current;
> + struct domain *currd = curr->domain;
> + struct vmcb_struct *vmcb = curr->arch.hvm_svm.vmcb;
> + u32 icrh = vmcb->exitinfo1 >> 32;
> + u32 icrl = vmcb->exitinfo1;
> + u32 id = vmcb->exitinfo2 >> 32;
> + u32 index = vmcb->exitinfo2 && 0xFF;
> +
> + switch ( id )
> + {
> + case AVIC_INCMP_IPI_ERR_INVALID_INT_TYPE:
> + /*
> + * AVIC hardware handles the delivery of
> + * IPIs when the specified Message Type is Fixed
> + * (also known as fixed delivery mode) and
> + * the Trigger Mode is edge-triggered. The hardware
> + * also supports self and broadcast delivery modes
> + * specified via the Destination Shorthand(DSH)
> + * field of the ICRL. Logical and physical APIC ID
> + * formats are supported. All other IPI types cause
> + * a #VMEXIT, which needs to emulated.
> + */
> + vlapic_reg_write(curr, APIC_ICR2, icrh);
> + vlapic_reg_write(curr, APIC_ICR, icrl);
> + break;
> +
> + case AVIC_INCMP_IPI_ERR_TARGET_NOT_RUN:
> + {
> + /*
> + * At this point, we expect that the AVIC HW has already
> + * set the appropriate IRR bits on the valid target
> + * vcpus. So, we just need to kick the appropriate vcpu.
> + */
> + struct vcpu *v;
> + uint32_t dest = GET_xAPIC_DEST_FIELD(icrh);
> + uint32_t short_hand = icrl & APIC_SHORT_MASK;
> + bool dest_mode = !!(icrl & APIC_DEST_MASK);
No need for !!. It is the explicit behaviour of the bool type.
> +
> + for_each_vcpu ( currd, v )
> + {
> + if ( v != curr &&
> + vlapic_match_dest(vcpu_vlapic(v), vcpu_vlapic(curr),
> + short_hand, dest, dest_mode) )
> + {
> + vcpu_kick(v);
> + break;
> + }
> + }
> + break;
> + }
> +
> + case AVIC_INCMP_IPI_ERR_INV_TARGET:
> + gprintk(XENLOG_ERR,
> + "SVM: %s: Invalid IPI target (icr=%#08x:%08x, idx=%u)\n",
> + __func__, icrh, icrl, index);
> + domain_crash(currd);
> + break;
> +
> + case AVIC_INCMP_IPI_ERR_INV_BK_PAGE:
> + gprintk(XENLOG_ERR,
> + "SVM: %s: Invalid bk page (icr=%#08x:%08x, idx=%u)\n",
> + __func__, icrh, icrl, index);
> + domain_crash(currd);
> + break;
> +
> + default:
> + gprintk(XENLOG_ERR, "SVM: %s: Unknown IPI interception (%#x)\n",
> + __func__, id);
> + domain_crash(currd);
> + }
> +}
> +
> +static struct avic_logical_id_entry *
> +avic_get_logical_id_entry(struct svm_domain *d, u32 ldr, bool flat)
> +{
> + unsigned int index;
> + unsigned int dest_id = GET_xAPIC_LOGICAL_ID(ldr);
> +
> + if ( !dest_id )
> + return NULL;
> +
> + if ( flat )
> + {
> + index = ffs(dest_id) - 1;
> + if ( index > 7 )
> + return NULL;
> + }
> + else
> + {
> + unsigned int cluster = (dest_id & 0xf0) >> 4;
> + int apic = ffs(dest_id & 0x0f) - 1;
> +
> + if ( (apic < 0) || (apic > 7) || (cluster >= 0xf) )
> + return NULL;
> + index = (cluster << 2) + apic;
> + }
> +
> + ASSERT(index <= 255);
> +
> + return &d->avic_logical_id_table[index];
> +}
> +
> +static int avic_ldr_write(struct vcpu *v, u8 g_phy_id, u32 ldr, bool valid)
> +{
> + struct avic_logical_id_entry *entry, new_entry;
> + u32 dfr = vlapic_read_aligned(vcpu_vlapic(v), APIC_DFR);
> +
> + entry = avic_get_logical_id_entry(&v->domain->arch.hvm_domain.svm,
> + ldr, (dfr == APIC_DFR_FLAT));
> + if (!entry)
if ( !entry )
> + return -EINVAL;
> +
> + new_entry = *entry;
> + smp_rmb();
> + new_entry.guest_phy_apic_id = g_phy_id;
> + new_entry.valid = valid;
> + *entry = new_entry;
> + smp_wmb();
These barriers don't do what you want. The pattern you are looking for
would require an smp_mb() between setting valid and writing things
back. However, that is overkill - all that matters is that the compiler
doesn't generate multiple partial updates.
new_entry.raw = ACCESS_ONCE(entry->raw);
new_entry.guest_phy_apic_id = g_phy_id;
new_entry.valid = valid;
ACCESS_ONCE(entry->raw) = new_entry.raw;
> +
> + return 0;
> +}
> +
> +static int avic_handle_ldr_update(struct vcpu *v)
> +{
> + int ret = 0;
> + u32 ldr = vlapic_read_aligned(vcpu_vlapic(v), APIC_LDR);
> + u32 apic_id = vlapic_read_aligned(vcpu_vlapic(v), APIC_ID);
> +
> + if ( !ldr )
> + return -EINVAL;
> +
> + ret = avic_ldr_write(v, GET_xAPIC_ID(apic_id), ldr, true);
> + if ( ret && v->arch.hvm_svm.avic_last_ldr )
> + {
> + /*
> + * Note:
> + * In case of failure to update LDR register,
> + * we set the guest physical APIC ID to 0,
> + * and set the entry logical APID ID entry
> + * to invalid (false).
> + */
> + avic_ldr_write(v, 0, v->arch.hvm_svm.avic_last_ldr, false);
> + v->arch.hvm_svm.avic_last_ldr = 0;
> + }
> + else
> + {
> + /*
> + * Note:
> + * This saves the last valid LDR so that we
> + * know which entry in the local APIC ID
> + * to clean up when the LDR is updated.
> + */
> + v->arch.hvm_svm.avic_last_ldr = ldr;
> + }
> +
> + return ret;
> +}
> +
> +static int avic_handle_dfr_update(struct vcpu *v)
> +{
> + u32 mod;
> + struct svm_domain *d = &v->domain->arch.hvm_domain.svm;
> + u32 dfr = vlapic_read_aligned(vcpu_vlapic(v), APIC_DFR);
> +
> + mod = (dfr >> 28) & 0xFu;
> +
> + spin_lock(&d->avic_dfr_mode_lock);
> + if ( d->avic_dfr_mode != mod )
> + {
> + /*
> + * We assume that all local APICs are using the same type.
> + * If DFR mode changes, we need to flush the domain AVIC logical
> + * APIC id table.
> + */
> + clear_domain_page(_mfn(page_to_mfn(d->avic_logical_id_table_pg)));
> + d->avic_dfr_mode = mod;
> + }
> + spin_unlock(&d->avic_dfr_mode_lock);
> +
> + if ( v->arch.hvm_svm.avic_last_ldr )
> + avic_handle_ldr_update(v);
> +
> + return 0;
> +}
> +
> +static int avic_unaccel_trap_write(struct vcpu *v)
> +{
> + struct vmcb_struct *vmcb = v->arch.hvm_svm.vmcb;
> + u32 offset = vmcb->exitinfo1 & AVIC_UNACCEL_ACCESS_OFFSET_MASK;
> + u32 reg = vlapic_read_aligned(vcpu_vlapic(v), offset);
> +
> + switch ( offset )
> + {
> + case APIC_ID:
> + /*
> + * Currently, we do not support APIC_ID update while
> + * the vcpus are running, which might require updating
> + * AVIC max APIC ID in all VMCBs. This would require
> + * synchronize update on all running VCPUs.
> + */
> + return X86EMUL_UNHANDLEABLE;
Newline please.
> + case APIC_LDR:
> + if ( avic_handle_ldr_update(v) )
> + return X86EMUL_UNHANDLEABLE;
> + break;
> +
> + case APIC_DFR:
> + if ( avic_handle_dfr_update(v) )
> + return X86EMUL_UNHANDLEABLE;
> + break;
> +
> + default:
> + break;
> + }
> +
> + vlapic_reg_write(v, offset, reg);
> +
> + return X86EMUL_OKAY;
> +}
> +
> +static inline bool avic_is_trap(u32 offset)
> +{
> + u32 pos = offset >> 4;
> + static const unsigned long avic_trap[] =
> + {
> +#define REG(x) (1UL << (APIC_ ## x >> 4))
> + REG(ID) | REG(EOI) | REG(RRR) | REG(LDR) |
> + REG(DFR) | REG(SPIV) | REG(ESR) | REG(ICR) |
> + REG(LVTT) | REG(LVTTHMR) | REG(LVTPC) | REG(LVT0) |
> + REG(LVT1) | REG(LVTERR) | REG(TMICT) | REG(TDCR)
> +#undef REG
> + };
I know I'm the author of the piece of code you've copied here, but I am
in the process of trying to fix its style.
static const unsigned long avic_trap[] = {
#define REG(x) (1UL << (APIC_ ## x >> 4))
REG(ID) | REG(EOI) | REG(RRR) | REG(LDR) |
....
#undef REG
};
> +
> + if ( !test_bit(pos, avic_trap) )
> + return false;
> + return true;
return pos < (sizeof(avic_trap) * 8) && test_bit(pos, avic_trap);
You need to avoid reading beyond the end of avic_trap[].
~Andrew
_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxxx
https://lists.xenproject.org/mailman/listinfo/xen-devel
|
 |
Lists.xenproject.org is hosted with RackSpace, monitoring our |