| Message ID | 20220221021922.733373-6-suravee.suthikulpanit@amd.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | Introducing AMD x2APIC Virtualization (x2AVIC) support. | expand |
On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote: > xAVIC and x2AVIC modes can support diffferent number of vcpus. > Update existing logics to support each mode accordingly. > > Also, modify the maximum physical APIC ID for AVIC to 255 to reflect > the actual value supported by the architecture. > > Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> > --- > arch/x86/include/asm/svm.h | 12 +++++++++--- > arch/x86/kvm/svm/avic.c | 8 +++++--- > 2 files changed, 14 insertions(+), 6 deletions(-) > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h > index 7a7a2297165b..681a348a9365 100644 > --- a/arch/x86/include/asm/svm.h > +++ b/arch/x86/include/asm/svm.h > @@ -250,10 +250,16 @@ enum avic_ipi_failure_cause { > > > /* > - * 0xff is broadcast, so the max index allowed for physical APIC ID > - * table is 0xfe. APIC IDs above 0xff are reserved. > + * For AVIC, the max index allowed for physical APIC ID > + * table is 0xff (255). > */ > -#define AVIC_MAX_PHYSICAL_ID_COUNT 0xff > +#define AVIC_MAX_PHYSICAL_ID 0XFFULL > + > +/* > + * For x2AVIC, the max index allowed for physical APIC ID > + * table is 0x1ff (511). > + */ > +#define X2AVIC_MAX_PHYSICAL_ID 0x1FFUL Yep, physid page can't hold more entries... This brings the inventible question of what to do when a VM has more that 512 vCPUs... With AVIC, since it is xapic, it would be easy - xapic supports up to 254 CPUs. But with x2apic, there is no such restriction on max 512 CPUs, thus it is legal to create a VM with x2apic and more that 512 CPUs, and x2AVIC won't work well in this case. I guess AVIC_IPI_FAILURE_INVALID_TARGET, has to be extened to support those cases, even with loss of performance, or we need to inhibit x2AVIC. Best regards, Maxim Levitsky > > #define AVIC_HPA_MASK ~((0xFFFULL << 52) | 0xFFF) > #define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL > diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c > index 0040824e4376..1999076966fd 100644 > --- a/arch/x86/kvm/svm/avic.c > +++ b/arch/x86/kvm/svm/avic.c > @@ -195,7 +195,7 @@ void avic_init_vmcb(struct vcpu_svm *svm) > vmcb->control.avic_backing_page = bpa & AVIC_HPA_MASK; > vmcb->control.avic_logical_id = lpa & AVIC_HPA_MASK; > vmcb->control.avic_physical_id = ppa & AVIC_HPA_MASK; > - vmcb->control.avic_physical_id |= AVIC_MAX_PHYSICAL_ID_COUNT; > + vmcb->control.avic_physical_id |= AVIC_MAX_PHYSICAL_ID; > vmcb->control.avic_vapic_bar = APIC_DEFAULT_PHYS_BASE & VMCB_AVIC_APIC_BAR_MASK; > > if (kvm_apicv_activated(svm->vcpu.kvm)) > @@ -210,7 +210,8 @@ static u64 *avic_get_physical_id_entry(struct kvm_vcpu *vcpu, > u64 *avic_physical_id_table; > struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm); > > - if (index >= AVIC_MAX_PHYSICAL_ID_COUNT) > + if ((avic_mode == AVIC_MODE_X1 && index > AVIC_MAX_PHYSICAL_ID) || > + (avic_mode == AVIC_MODE_X2 && index > X2AVIC_MAX_PHYSICAL_ID)) > return NULL; > > avic_physical_id_table = page_address(kvm_svm->avic_physical_id_table_page); > @@ -257,7 +258,8 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu) > int id = vcpu->vcpu_id; > struct vcpu_svm *svm = to_svm(vcpu); > > - if (id >= AVIC_MAX_PHYSICAL_ID_COUNT) > + if ((avic_mode == AVIC_MODE_X1 && id > AVIC_MAX_PHYSICAL_ID) || > + (avic_mode == AVIC_MODE_X2 && id > X2AVIC_MAX_PHYSICAL_ID)) > return -EINVAL; > > if (!vcpu->arch.apic->regs)
Hi Maxim, On 2/25/22 12:18 AM, Maxim Levitsky wrote: > On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote: >> xAVIC and x2AVIC modes can support diffferent number of vcpus. >> Update existing logics to support each mode accordingly. >> >> Also, modify the maximum physical APIC ID for AVIC to 255 to reflect >> the actual value supported by the architecture. >> >> Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com> >> --- >> arch/x86/include/asm/svm.h | 12 +++++++++--- >> arch/x86/kvm/svm/avic.c | 8 +++++--- >> 2 files changed, 14 insertions(+), 6 deletions(-) >> >> diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h >> index 7a7a2297165b..681a348a9365 100644 >> --- a/arch/x86/include/asm/svm.h >> +++ b/arch/x86/include/asm/svm.h >> @@ -250,10 +250,16 @@ enum avic_ipi_failure_cause { >> >> >> /* >> - * 0xff is broadcast, so the max index allowed for physical APIC ID >> - * table is 0xfe. APIC IDs above 0xff are reserved. >> + * For AVIC, the max index allowed for physical APIC ID >> + * table is 0xff (255). >> */ >> -#define AVIC_MAX_PHYSICAL_ID_COUNT 0xff >> +#define AVIC_MAX_PHYSICAL_ID 0XFFULL >> + >> +/* >> + * For x2AVIC, the max index allowed for physical APIC ID >> + * table is 0x1ff (511). >> + */ >> +#define X2AVIC_MAX_PHYSICAL_ID 0x1FFUL > Yep, physid page can't hold more entries... > > This brings the inventible question of what to do when a VM has more > that 512 vCPUs... > > With AVIC, since it is xapic, it would be easy - xapic supports up to > 254 CPUs. Actually, 255 vCPUs. > But with x2apic, there is no such restriction on max 512 CPUs, > thus it is legal to create a VM with x2apic and more that 512 CPUs, > and x2AVIC won't work well in this case. > > I guess AVIC_IPI_FAILURE_INVALID_TARGET, has to be extened to support those > cases, even with loss of performance, or we need to inhibit x2AVIC. In case of x2APIC-enabled guest w/ vCPU exceeding the max APIC ID (512) limit, the ioctl operation for KVM_CREATE_VCPU will fail. For QEMU, this would exit with error code. Would this be sufficient? Regards, Suravee
On Tue, 2022-03-01 at 17:47 +0700, Suravee Suthikulpanit wrote: > Hi Maxim, > > On 2/25/22 12:18 AM, Maxim Levitsky wrote: > > On Sun, 2022-02-20 at 20:19 -0600, Suravee Suthikulpanit wrote: > > > xAVIC and x2AVIC modes can support diffferent number of vcpus. > > > Update existing logics to support each mode accordingly. > > > > > > Also, modify the maximum physical APIC ID for AVIC to 255 to reflect > > > the actual value supported by the architecture. > > > > > > Signed-off-by: Suravee Suthikulpanit<suravee.suthikulpanit@amd.com> > > > --- > > > arch/x86/include/asm/svm.h | 12 +++++++++--- > > > arch/x86/kvm/svm/avic.c | 8 +++++--- > > > 2 files changed, 14 insertions(+), 6 deletions(-) > > > > > > diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h > > > index 7a7a2297165b..681a348a9365 100644 > > > --- a/arch/x86/include/asm/svm.h > > > +++ b/arch/x86/include/asm/svm.h > > > @@ -250,10 +250,16 @@ enum avic_ipi_failure_cause { > > > > > > > > > /* > > > - * 0xff is broadcast, so the max index allowed for physical APIC ID > > > - * table is 0xfe. APIC IDs above 0xff are reserved. > > > + * For AVIC, the max index allowed for physical APIC ID > > > + * table is 0xff (255). > > > */ > > > -#define AVIC_MAX_PHYSICAL_ID_COUNT 0xff > > > +#define AVIC_MAX_PHYSICAL_ID 0XFFULL > > > + > > > +/* > > > + * For x2AVIC, the max index allowed for physical APIC ID > > > + * table is 0x1ff (511). > > > + */ > > > +#define X2AVIC_MAX_PHYSICAL_ID 0x1FFUL > > Yep, physid page can't hold more entries... > > > > This brings the inventible question of what to do when a VM has more > > that 512 vCPUs... > > > > With AVIC, since it is xapic, it would be easy - xapic supports up to > > 254 CPUs. > > Actually, 255 vCPUs. Sorry for off-by-one mistake - just remembered that 0xFF is reserved, but then 255 is already 1 less that 256. > > > But with x2apic, there is no such restriction on max 512 CPUs, > > thus it is legal to create a VM with x2apic and more that 512 CPUs, > > and x2AVIC won't work well in this case. > > > > I guess AVIC_IPI_FAILURE_INVALID_TARGET, has to be extened to support those > > cases, even with loss of performance, or we need to inhibit x2AVIC. > > In case of x2APIC-enabled guest w/ vCPU exceeding the max APIC ID (512) limit, > the ioctl operation for KVM_CREATE_VCPU will fail. For QEMU, this would > exit with error code. Would this be sufficient? Yes, this is the best. Best regards, Maxim Levitsky > > Regards, > Suravee > > >
diff --git a/arch/x86/include/asm/svm.h b/arch/x86/include/asm/svm.h index 7a7a2297165b..681a348a9365 100644 --- a/arch/x86/include/asm/svm.h +++ b/arch/x86/include/asm/svm.h @@ -250,10 +250,16 @@ enum avic_ipi_failure_cause { /* - * 0xff is broadcast, so the max index allowed for physical APIC ID - * table is 0xfe. APIC IDs above 0xff are reserved. + * For AVIC, the max index allowed for physical APIC ID + * table is 0xff (255). */ -#define AVIC_MAX_PHYSICAL_ID_COUNT 0xff +#define AVIC_MAX_PHYSICAL_ID 0XFFULL + +/* + * For x2AVIC, the max index allowed for physical APIC ID + * table is 0x1ff (511). + */ +#define X2AVIC_MAX_PHYSICAL_ID 0x1FFUL #define AVIC_HPA_MASK ~((0xFFFULL << 52) | 0xFFF) #define VMCB_AVIC_APIC_BAR_MASK 0xFFFFFFFFFF000ULL diff --git a/arch/x86/kvm/svm/avic.c b/arch/x86/kvm/svm/avic.c index 0040824e4376..1999076966fd 100644 --- a/arch/x86/kvm/svm/avic.c +++ b/arch/x86/kvm/svm/avic.c @@ -195,7 +195,7 @@ void avic_init_vmcb(struct vcpu_svm *svm) vmcb->control.avic_backing_page = bpa & AVIC_HPA_MASK; vmcb->control.avic_logical_id = lpa & AVIC_HPA_MASK; vmcb->control.avic_physical_id = ppa & AVIC_HPA_MASK; - vmcb->control.avic_physical_id |= AVIC_MAX_PHYSICAL_ID_COUNT; + vmcb->control.avic_physical_id |= AVIC_MAX_PHYSICAL_ID; vmcb->control.avic_vapic_bar = APIC_DEFAULT_PHYS_BASE & VMCB_AVIC_APIC_BAR_MASK; if (kvm_apicv_activated(svm->vcpu.kvm)) @@ -210,7 +210,8 @@ static u64 *avic_get_physical_id_entry(struct kvm_vcpu *vcpu, u64 *avic_physical_id_table; struct kvm_svm *kvm_svm = to_kvm_svm(vcpu->kvm); - if (index >= AVIC_MAX_PHYSICAL_ID_COUNT) + if ((avic_mode == AVIC_MODE_X1 && index > AVIC_MAX_PHYSICAL_ID) || + (avic_mode == AVIC_MODE_X2 && index > X2AVIC_MAX_PHYSICAL_ID)) return NULL; avic_physical_id_table = page_address(kvm_svm->avic_physical_id_table_page); @@ -257,7 +258,8 @@ static int avic_init_backing_page(struct kvm_vcpu *vcpu) int id = vcpu->vcpu_id; struct vcpu_svm *svm = to_svm(vcpu); - if (id >= AVIC_MAX_PHYSICAL_ID_COUNT) + if ((avic_mode == AVIC_MODE_X1 && id > AVIC_MAX_PHYSICAL_ID) || + (avic_mode == AVIC_MODE_X2 && id > X2AVIC_MAX_PHYSICAL_ID)) return -EINVAL; if (!vcpu->arch.apic->regs)
xAVIC and x2AVIC modes can support diffferent number of vcpus. Update existing logics to support each mode accordingly. Also, modify the maximum physical APIC ID for AVIC to 255 to reflect the actual value supported by the architecture. Signed-off-by: Suravee Suthikulpanit <suravee.suthikulpanit@amd.com> --- arch/x86/include/asm/svm.h | 12 +++++++++--- arch/x86/kvm/svm/avic.c | 8 +++++--- 2 files changed, 14 insertions(+), 6 deletions(-)