Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Joao Martins]

On 07/05/2016 04:44 PM, Jan Beulich wrote:
 On 05.07.16 at 17:34,  wrote:
>> On Thu, Jun 30, 2016 at 03:10:11AM -0600, Jan Beulich wrote:
>> On 29.06.16 at 18:27,  wrote:
 On 29/06/16 17:19, Vitaly Kuznetsov wrote:
> To explain better what I'm trying to suggest here please take a look at
> the attached patch. If we can guarantee long term that ACPI id always
> equals to Xen's idea of vCPU id this is probably the easiest way.
>
> -- Vitaly

 The code in hvmloader which sets up the MADT does:

 for ( i = 0; i < hvm_info->nr_vcpus; i++ )
 {
 memset(lapic, 0, sizeof(*lapic));
 lapic->type= ACPI_PROCESSOR_LOCAL_APIC;
 lapic->length  = sizeof(*lapic);
 /* Processor ID must match processor-object IDs in the DSDT. */
 lapic->acpi_processor_id = i;
 lapic->apic_id = LAPIC_ID(i);
 lapic->flags = (test_bit(i, hvm_info->vcpu_online)
 ? ACPI_LOCAL_APIC_ENABLED : 0);
 lapic++;
 }

 So relying on the acpi_processor_id does look to be reliable.  That code
 hasn't changed since 2007, and that was only a bugfix.  I would go so
 far as to say it is reasonable for us to guarantee this in the guest ABI.
>>>
>>> In fact - is there any other way a guest could learn the vCPU IDs
>>> of its CPUs in a reliable way? I don't think so, and hence this de
>>> facto already is part of the ABI; we should of course spell it out
>>> somewhere.
>>
>> CCing Joao.
>>
>> Joao worked (and I think he posted an RFC patchset?) where this is changed so
>> that the true hardware topology (core, thread, etc) is exposed. This is 
>> obviously
>> for cases where you want pinning.
>>
>> I would hesistate to spell this out as an ABI..
> 
> Are you perhaps mixing up ACPI and APIC IDs? Here talk is of the
> former, while Joao's patch set was about the latter iirc.
> 
Correct, my patch series indeed changed APIC IDs. I guess making 
acpi_processor_id to
get vcpu_id reliably stated as guest ABI, is safe wrt to future arrangements to 
apic_ids.

Joao

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Jan Beulich]

>>> On 05.07.16 at 17:34,  wrote:
> On Thu, Jun 30, 2016 at 03:10:11AM -0600, Jan Beulich wrote:
>> >>> On 29.06.16 at 18:27,  wrote:
>> > On 29/06/16 17:19, Vitaly Kuznetsov wrote:
>> >> To explain better what I'm trying to suggest here please take a look at
>> >> the attached patch. If we can guarantee long term that ACPI id always
>> >> equals to Xen's idea of vCPU id this is probably the easiest way.
>> >>
>> >> -- Vitaly
>> > 
>> > The code in hvmloader which sets up the MADT does:
>> > 
>> > for ( i = 0; i < hvm_info->nr_vcpus; i++ )
>> > {
>> > memset(lapic, 0, sizeof(*lapic));
>> > lapic->type= ACPI_PROCESSOR_LOCAL_APIC;
>> > lapic->length  = sizeof(*lapic);
>> > /* Processor ID must match processor-object IDs in the DSDT. */
>> > lapic->acpi_processor_id = i;
>> > lapic->apic_id = LAPIC_ID(i);
>> > lapic->flags = (test_bit(i, hvm_info->vcpu_online)
>> > ? ACPI_LOCAL_APIC_ENABLED : 0);
>> > lapic++;
>> > }
>> > 
>> > So relying on the acpi_processor_id does look to be reliable.  That code
>> > hasn't changed since 2007, and that was only a bugfix.  I would go so
>> > far as to say it is reasonable for us to guarantee this in the guest ABI.
>> 
>> In fact - is there any other way a guest could learn the vCPU IDs
>> of its CPUs in a reliable way? I don't think so, and hence this de
>> facto already is part of the ABI; we should of course spell it out
>> somewhere.
> 
> CCing Joao.
> 
> Joao worked (and I think he posted an RFC patchset?) where this is changed so
> that the true hardware topology (core, thread, etc) is exposed. This is 
> obviously
> for cases where you want pinning.
> 
> I would hesistate to spell this out as an ABI..

Are you perhaps mixing up ACPI and APIC IDs? Here talk is of the
former, while Joao's patch set was about the latter iirc.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Konrad Rzeszutek Wilk]

On Thu, Jun 30, 2016 at 03:10:11AM -0600, Jan Beulich wrote:
> >>> On 29.06.16 at 18:27,  wrote:
> > On 29/06/16 17:19, Vitaly Kuznetsov wrote:
> >> To explain better what I'm trying to suggest here please take a look at
> >> the attached patch. If we can guarantee long term that ACPI id always
> >> equals to Xen's idea of vCPU id this is probably the easiest way.
> >>
> >> -- Vitaly
> > 
> > The code in hvmloader which sets up the MADT does:
> > 
> > for ( i = 0; i < hvm_info->nr_vcpus; i++ )
> > {
> > memset(lapic, 0, sizeof(*lapic));
> > lapic->type= ACPI_PROCESSOR_LOCAL_APIC;
> > lapic->length  = sizeof(*lapic);
> > /* Processor ID must match processor-object IDs in the DSDT. */
> > lapic->acpi_processor_id = i;
> > lapic->apic_id = LAPIC_ID(i);
> > lapic->flags = (test_bit(i, hvm_info->vcpu_online)
> > ? ACPI_LOCAL_APIC_ENABLED : 0);
> > lapic++;
> > }
> > 
> > So relying on the acpi_processor_id does look to be reliable.  That code
> > hasn't changed since 2007, and that was only a bugfix.  I would go so
> > far as to say it is reasonable for us to guarantee this in the guest ABI.
> 
> In fact - is there any other way a guest could learn the vCPU IDs
> of its CPUs in a reliable way? I don't think so, and hence this de
> facto already is part of the ABI; we should of course spell it out
> somewhere.

CCing Joao.

Joao worked (and I think he posted an RFC patchset?) where this is changed so
that the true hardware topology (core, thread, etc) is exposed. This is 
obviously
for cases where you want pinning.

I would hesistate to spell this out as an ABI..

P.S.
Which reminds me, Joao, you OK posting the patchset?
> 
> Jan
> 
> 
> ___
> Xen-devel mailing list
> Xen-devel@lists.xen.org
> http://lists.xen.org/xen-devel

___
Xen-devel mailing list
Xen-devel@lists.xen.org
https://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Jan Beulich]

>>> On 01.07.16 at 14:06,  wrote:
> "Jan Beulich"  writes:
> 
> On 29.06.16 at 18:27,  wrote:
>>> On 29/06/16 17:19, Vitaly Kuznetsov wrote:
 To explain better what I'm trying to suggest here please take a look at
 the attached patch. If we can guarantee long term that ACPI id always
 equals to Xen's idea of vCPU id this is probably the easiest way.

 -- Vitaly
>>> 
>>> The code in hvmloader which sets up the MADT does:
>>> 
>>> for ( i = 0; i < hvm_info->nr_vcpus; i++ )
>>> {
>>> memset(lapic, 0, sizeof(*lapic));
>>> lapic->type= ACPI_PROCESSOR_LOCAL_APIC;
>>> lapic->length  = sizeof(*lapic);
>>> /* Processor ID must match processor-object IDs in the DSDT. */
>>> lapic->acpi_processor_id = i;
>>> lapic->apic_id = LAPIC_ID(i);
>>> lapic->flags = (test_bit(i, hvm_info->vcpu_online)
>>> ? ACPI_LOCAL_APIC_ENABLED : 0);
>>> lapic++;
>>> }
>>> 
>>> So relying on the acpi_processor_id does look to be reliable.  That code
>>> hasn't changed since 2007, and that was only a bugfix.  I would go so
>>> far as to say it is reasonable for us to guarantee this in the guest ABI.
>>
>> In fact - is there any other way a guest could learn the vCPU IDs
>> of its CPUs in a reliable way? I don't think so, and hence this de
>> facto already is part of the ABI; we should of course spell it out
>> somewhere.
> 
> I'm unsure about the right place in the hypervisor tree to put this
> information to. At the very least users of VCPUOP_* and EVTCHNOP_*
> hypervcalls need to know this so xen/include/public/{event_channel.h,
> vcpu.h} are the candidates. Or is there a better place?

I'd probably put this in public/hvm/hvm_info_table.h or
public/hvm/hvm_vcpu.h; the two headers you name shouldn't be
cluttered with information like this not pertaining to all guests.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

"Jan Beulich"  writes:

 On 29.06.16 at 18:27,  wrote:
>> On 29/06/16 17:19, Vitaly Kuznetsov wrote:
>>> To explain better what I'm trying to suggest here please take a look at
>>> the attached patch. If we can guarantee long term that ACPI id always
>>> equals to Xen's idea of vCPU id this is probably the easiest way.
>>>
>>> -- Vitaly
>> 
>> The code in hvmloader which sets up the MADT does:
>> 
>> for ( i = 0; i < hvm_info->nr_vcpus; i++ )
>> {
>> memset(lapic, 0, sizeof(*lapic));
>> lapic->type= ACPI_PROCESSOR_LOCAL_APIC;
>> lapic->length  = sizeof(*lapic);
>> /* Processor ID must match processor-object IDs in the DSDT. */
>> lapic->acpi_processor_id = i;
>> lapic->apic_id = LAPIC_ID(i);
>> lapic->flags = (test_bit(i, hvm_info->vcpu_online)
>> ? ACPI_LOCAL_APIC_ENABLED : 0);
>> lapic++;
>> }
>> 
>> So relying on the acpi_processor_id does look to be reliable.  That code
>> hasn't changed since 2007, and that was only a bugfix.  I would go so
>> far as to say it is reasonable for us to guarantee this in the guest ABI.
>
> In fact - is there any other way a guest could learn the vCPU IDs
> of its CPUs in a reliable way? I don't think so, and hence this de
> facto already is part of the ABI; we should of course spell it out
> somewhere.

I'm unsure about the right place in the hypervisor tree to put this
information to. At the very least users of VCPUOP_* and EVTCHNOP_*
hypervcalls need to know this so xen/include/public/{event_channel.h,
vcpu.h} are the candidates. Or is there a better place?

-- 
  Vitaly

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Jan Beulich]

>>> On 29.06.16 at 18:27,  wrote:
> On 29/06/16 17:19, Vitaly Kuznetsov wrote:
>> To explain better what I'm trying to suggest here please take a look at
>> the attached patch. If we can guarantee long term that ACPI id always
>> equals to Xen's idea of vCPU id this is probably the easiest way.
>>
>> -- Vitaly
> 
> The code in hvmloader which sets up the MADT does:
> 
> for ( i = 0; i < hvm_info->nr_vcpus; i++ )
> {
> memset(lapic, 0, sizeof(*lapic));
> lapic->type= ACPI_PROCESSOR_LOCAL_APIC;
> lapic->length  = sizeof(*lapic);
> /* Processor ID must match processor-object IDs in the DSDT. */
> lapic->acpi_processor_id = i;
> lapic->apic_id = LAPIC_ID(i);
> lapic->flags = (test_bit(i, hvm_info->vcpu_online)
> ? ACPI_LOCAL_APIC_ENABLED : 0);
> lapic++;
> }
> 
> So relying on the acpi_processor_id does look to be reliable.  That code
> hasn't changed since 2007, and that was only a bugfix.  I would go so
> far as to say it is reasonable for us to guarantee this in the guest ABI.

In fact - is there any other way a guest could learn the vCPU IDs
of its CPUs in a reliable way? I don't think so, and hence this de
facto already is part of the ABI; we should of course spell it out
somewhere.

Jan


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Andrew Cooper]

On 29/06/16 17:19, Vitaly Kuznetsov wrote:
> Vitaly Kuznetsov  writes:
>
>> > Andrew Cooper  writes:
>> >
>>> >> On 29/06/16 13:16, Vitaly Kuznetsov wrote:
 >>> Andrew Cooper  writes:
 >>>
>  On 28/06/16 17:47, Vitaly Kuznetsov wrote:
>> > @@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct 
>> > notifier_block *self, unsigned long action,
>> >int cpu = (long)hcpu;
>> >switch (action) {
>> >case CPU_UP_PREPARE:
>> > +  /* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
>> > +  per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;
>  Please do not assume or propagate this brokenness.  It is incorrect 
>  in
>  the general case, and I will be fixing in the hypervisor in due 
>  course.
> 
>  Always read the APIC_ID from the LAPIC, per regular hardware.
 >>> (I'm probbaly missing something important - please bear with me)
 >>>
 >>> The problem here is that I need to get _other_ CPU's id before any code
 >>> is executed on that CPU (or, at least, this is the current state of
 >>> affairs if you look at xen_hvm_cpu_up()) so I can't use CPUID/do MSR
 >>> reads/... The only option I see here is to rely on ACPI (MADT) data
 >>> which is stored in x86_cpu_to_apicid (and that's what cpu_physical_id()
 >>> gives us). MADT also has processor id which connects it to DSDT but I'm
 >>> not sure Linux keeps this data. But this is something fixable I guess.
>>> >>
>>> >> Hmm yes - that is a tricky issue.
>>> >>
>>> >> It is not safe or correct to assume that xen_vcpu_id is APICID / 2.
>>> >>
>>> >> This is currently the case for most modern versions of Xen, but isn't
>>> >> the case for older versions, and won't be the case in the future when I
>>> >> (or someone else) fixes topology representation for guests.
>>> >>
>>> >> For this to work, we need one or more of:
>>> >>
>>> >> 1) to provide the guest a full mapping from APIC_ID to vcpu id at boot
>>> >> time.
>> >
>> > So can we rely on ACPI data? Especially on MADT and processor ids there?
>> > I think we can always guarantee that processor ids there match vCPU
>> > ids. If yes I can try saving this data when we parse MADT.
>> >
> To explain better what I'm trying to suggest here please take a look at
> the attached patch. If we can guarantee long term that ACPI id always
> equals to Xen's idea of vCPU id this is probably the easiest way.
>
> -- Vitaly

The code in hvmloader which sets up the MADT does:

for ( i = 0; i < hvm_info->nr_vcpus; i++ )
{
memset(lapic, 0, sizeof(*lapic));
lapic->type= ACPI_PROCESSOR_LOCAL_APIC;
lapic->length  = sizeof(*lapic);
/* Processor ID must match processor-object IDs in the DSDT. */
lapic->acpi_processor_id = i;
lapic->apic_id = LAPIC_ID(i);
lapic->flags = (test_bit(i, hvm_info->vcpu_online)
? ACPI_LOCAL_APIC_ENABLED : 0);
lapic++;
}

So relying on the acpi_processor_id does look to be reliable.  That code
hasn't changed since 2007, and that was only a bugfix.  I would go so
far as to say it is reasonable for us to guarantee this in the guest ABI.

Jan - thoughts?

~Andrew
___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

Vitaly Kuznetsov  writes:

> Andrew Cooper  writes:
>
>> On 29/06/16 13:16, Vitaly Kuznetsov wrote:
>>> Andrew Cooper  writes:
>>>
 On 28/06/16 17:47, Vitaly Kuznetsov wrote:
> @@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct notifier_block 
> *self, unsigned long action,
>   int cpu = (long)hcpu;
>   switch (action) {
>   case CPU_UP_PREPARE:
> + /* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
> + per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;
 Please do not assume or propagate this brokenness.  It is incorrect in
 the general case, and I will be fixing in the hypervisor in due course.

 Always read the APIC_ID from the LAPIC, per regular hardware.
>>> (I'm probbaly missing something important - please bear with me)
>>>
>>> The problem here is that I need to get _other_ CPU's id before any code
>>> is executed on that CPU (or, at least, this is the current state of
>>> affairs if you look at xen_hvm_cpu_up()) so I can't use CPUID/do MSR
>>> reads/... The only option I see here is to rely on ACPI (MADT) data
>>> which is stored in x86_cpu_to_apicid (and that's what cpu_physical_id()
>>> gives us). MADT also has processor id which connects it to DSDT but I'm
>>> not sure Linux keeps this data. But this is something fixable I guess.
>>
>> Hmm yes - that is a tricky issue.
>>
>> It is not safe or correct to assume that xen_vcpu_id is APICID / 2.
>>
>> This is currently the case for most modern versions of Xen, but isn't
>> the case for older versions, and won't be the case in the future when I
>> (or someone else) fixes topology representation for guests.
>>
>> For this to work, we need one or more of:
>>
>> 1) to provide the guest a full mapping from APIC_ID to vcpu id at boot
>> time.
>
> So can we rely on ACPI data? Especially on MADT and processor ids there?
> I think we can always guarantee that processor ids there match vCPU
> ids. If yes I can try saving this data when we parse MADT.
>

To explain better what I'm trying to suggest here please take a look at
the attached patch. If we can guarantee long term that ACPI id always
equals to Xen's idea of vCPU id this is probably the easiest way.

-- 
  Vitaly

>From 7c9cd25bdcd3e6ee866aa49550c9a4160194c3ba Mon Sep 17 00:00:00 2001
From: Vitaly Kuznetsov 
Date: Wed, 29 Jun 2016 18:13:01 +0200
Subject: [PATCH RFC/WIP] x86/acpi: store APIC ids for future usage (+use it in
 xen_hvm_cpu_notify)

Signed-off-by: Vitaly Kuznetsov 
---
 arch/x86/include/asm/smp.h |  1 +
 arch/x86/kernel/acpi/boot.c| 18 ++
 arch/x86/kernel/setup_percpu.c |  3 +++
 arch/x86/xen/enlighten.c   |  8 ++--
 4 files changed, 24 insertions(+), 6 deletions(-)

diff --git a/arch/x86/include/asm/smp.h b/arch/x86/include/asm/smp.h
index 66b0573..c68b56a 100644
--- a/arch/x86/include/asm/smp.h
+++ b/arch/x86/include/asm/smp.h
@@ -33,6 +33,7 @@ static inline struct cpumask *cpu_llc_shared_mask(int cpu)
 }
 
 DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_cpu_to_apicid);
+DECLARE_EARLY_PER_CPU_READ_MOSTLY(int, x86_cpu_to_acpiid);
 DECLARE_EARLY_PER_CPU_READ_MOSTLY(u16, x86_bios_cpu_apicid);
 #if defined(CONFIG_X86_LOCAL_APIC) && defined(CONFIG_X86_32)
 DECLARE_EARLY_PER_CPU_READ_MOSTLY(int, x86_cpu_to_logical_apicid);
diff --git a/arch/x86/kernel/acpi/boot.c b/arch/x86/kernel/acpi/boot.c
index 9414f84..3bbf0ab 100644
--- a/arch/x86/kernel/acpi/boot.c
+++ b/arch/x86/kernel/acpi/boot.c
@@ -110,6 +110,9 @@ static u32 isa_irq_to_gsi[NR_IRQS_LEGACY] __read_mostly = {
 
 #define	ACPI_INVALID_GSI		INT_MIN
 
+DEFINE_EARLY_PER_CPU_READ_MOSTLY(int, x86_cpu_to_acpiid, -1);
+EXPORT_EARLY_PER_CPU_SYMBOL(x86_cpu_to_acpiid);
+
 /*
  * This is just a simple wrapper around early_ioremap(),
  * with sanity checks for phys == 0 and size == 0.
@@ -165,9 +168,10 @@ static int __init acpi_parse_madt(struct acpi_table_header *table)
  *
  * Returns the logic cpu number which maps to the local apic
  */
-static int acpi_register_lapic(int id, u8 enabled)
+static int acpi_register_lapic(int id, int acpiid, u8 enabled)
 {
 	unsigned int ver = 0;
+	int cpu;
 
 	if (id >= MAX_LOCAL_APIC) {
 		printk(KERN_INFO PREFIX "skipped apicid that is too big\n");
@@ -182,7 +186,11 @@ static int acpi_register_lapic(int id, u8 enabled)
 	if (boot_cpu_physical_apicid != -1U)
 		ver = apic_version[boot_cpu_physical_apicid];
 
-	return generic_processor_info(id, ver);
+	cpu = generic_processor_info(id, ver);
+	if (cpu >= 0)
+		early_per_cpu(x86_cpu_to_acpiid, cpu) = acpiid;
+
+	return cpu;
 }
 
 static int __init
@@ -212,7 +220,7 @@ acpi_parse_x2apic(struct acpi_subtable_header *header, const unsigned long end)
 	if (!apic->apic_id_valid(apic_id) && enabled)
 		printk(KERN_WARNING PREFIX "x2apic entry ignored\n");
 	else
-		acpi_register_lapic(apic_id, enabled);
+		acpi_register_lapic(apic_id, processor->uid, 

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

Andrew Cooper  writes:

> On 29/06/16 13:16, Vitaly Kuznetsov wrote:
>> Andrew Cooper  writes:
>>
>>> On 28/06/16 17:47, Vitaly Kuznetsov wrote:
 @@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct notifier_block 
 *self, unsigned long action,
int cpu = (long)hcpu;
switch (action) {
case CPU_UP_PREPARE:
 +  /* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
 +  per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;
>>> Please do not assume or propagate this brokenness.  It is incorrect in
>>> the general case, and I will be fixing in the hypervisor in due course.
>>>
>>> Always read the APIC_ID from the LAPIC, per regular hardware.
>> (I'm probbaly missing something important - please bear with me)
>>
>> The problem here is that I need to get _other_ CPU's id before any code
>> is executed on that CPU (or, at least, this is the current state of
>> affairs if you look at xen_hvm_cpu_up()) so I can't use CPUID/do MSR
>> reads/... The only option I see here is to rely on ACPI (MADT) data
>> which is stored in x86_cpu_to_apicid (and that's what cpu_physical_id()
>> gives us). MADT also has processor id which connects it to DSDT but I'm
>> not sure Linux keeps this data. But this is something fixable I guess.
>
> Hmm yes - that is a tricky issue.
>
> It is not safe or correct to assume that xen_vcpu_id is APICID / 2.
>
> This is currently the case for most modern versions of Xen, but isn't
> the case for older versions, and won't be the case in the future when I
> (or someone else) fixes topology representation for guests.
>
> For this to work, we need one or more of:
>
> 1) to provide the guest a full mapping from APIC_ID to vcpu id at boot
> time.

So can we rely on ACPI data? Especially on MADT and processor ids there?
I think we can always guarantee that processor ids there match vCPU
ids. If yes I can try saving this data when we parse MADT.

> 2) add a new interface where the guest can explicitly query "what is the
> vcpu id for the entity with this APIC_ID".
> 3) Allow HVM guests to identify a vcpu in a hypercall by APIC_ID.
>
> 3 is the cleaner approach, but given that vcpu ids have already leaked
> into an HVM domains idea of the world, 1 or 2 is probably a better
> ladder to dig us out of this hole.

It would be nice to avoid hypervisor changes but if we have to modify it
we can fail all secondary CPUs for now when we detect that CPU0's vCPU
id is not 0 (and CPU0 gets its id with CPUID).

-- 
  Vitaly

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Andrew Cooper]

On 29/06/16 13:16, Vitaly Kuznetsov wrote:
> Andrew Cooper  writes:
>
>> On 28/06/16 17:47, Vitaly Kuznetsov wrote:
>>> @@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct notifier_block 
>>> *self, unsigned long action,
>>> int cpu = (long)hcpu;
>>> switch (action) {
>>> case CPU_UP_PREPARE:
>>> +   /* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
>>> +   per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;
>> Please do not assume or propagate this brokenness.  It is incorrect in
>> the general case, and I will be fixing in the hypervisor in due course.
>>
>> Always read the APIC_ID from the LAPIC, per regular hardware.
> (I'm probbaly missing something important - please bear with me)
>
> The problem here is that I need to get _other_ CPU's id before any code
> is executed on that CPU (or, at least, this is the current state of
> affairs if you look at xen_hvm_cpu_up()) so I can't use CPUID/do MSR
> reads/... The only option I see here is to rely on ACPI (MADT) data
> which is stored in x86_cpu_to_apicid (and that's what cpu_physical_id()
> gives us). MADT also has processor id which connects it to DSDT but I'm
> not sure Linux keeps this data. But this is something fixable I guess.

Hmm yes - that is a tricky issue.

It is not safe or correct to assume that xen_vcpu_id is APICID / 2.

This is currently the case for most modern versions of Xen, but isn't
the case for older versions, and won't be the case in the future when I
(or someone else) fixes topology representation for guests.

For this to work, we need one or more of:

1) to provide the guest a full mapping from APIC_ID to vcpu id at boot time.
2) add a new interface where the guest can explicitly query "what is the
vcpu id for the entity with this APIC_ID".
3) Allow HVM guests to identify a vcpu in a hypercall by APIC_ID.

3 is the cleaner approach, but given that vcpu ids have already leaked
into an HVM domains idea of the world, 1 or 2 is probably a better
ladder to dig us out of this hole.

~Andrew.

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

Andrew Cooper  writes:

> On 28/06/16 17:47, Vitaly Kuznetsov wrote:
>> @@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct notifier_block 
>> *self, unsigned long action,
>>  int cpu = (long)hcpu;
>>  switch (action) {
>>  case CPU_UP_PREPARE:
>> +/* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
>> +per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;
>
> Please do not assume or propagate this brokenness.  It is incorrect in
> the general case, and I will be fixing in the hypervisor in due course.
>
> Always read the APIC_ID from the LAPIC, per regular hardware.

(I'm probbaly missing something important - please bear with me)

The problem here is that I need to get _other_ CPU's id before any code
is executed on that CPU (or, at least, this is the current state of
affairs if you look at xen_hvm_cpu_up()) so I can't use CPUID/do MSR
reads/... The only option I see here is to rely on ACPI (MADT) data
which is stored in x86_cpu_to_apicid (and that's what cpu_physical_id()
gives us). MADT also has processor id which connects it to DSDT but I'm
not sure Linux keeps this data. But this is something fixable I guess.

-- 
  Vitaly

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

Re: [Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Andrew Cooper]

On 28/06/16 17:47, Vitaly Kuznetsov wrote:
> @@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct notifier_block 
> *self, unsigned long action,
>   int cpu = (long)hcpu;
>   switch (action) {
>   case CPU_UP_PREPARE:
> + /* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
> + per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;

Please do not assume or propagate this brokenness.  It is incorrect in
the general case, and I will be fixing in the hypervisor in due course.

Always read the APIC_ID from the LAPIC, per regular hardware.

~Andrew

___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel

[Xen-devel] [PATCH linux 2/8] xen: introduce xen_vcpu_id mapping

[Vitaly Kuznetsov]

It may happen that Xen's and Linux's ideas of vCPU id diverge. In
particular, when we crash on a secondary vCPU we may want to do kdump
and unlike plain kexec where we do migrate_to_reboot_cpu() we try booting
on the vCPU which crashed. This doesn't work very well for PVHVM guests as
we have a number of hypercalls where we pass vCPU id as a parameter. These
hypercalls either fail or do something unexpected. To solve the issue
introduce percpu xen_vcpu_id mapping. ARM and PV guests get direct mapping
for now. Boot CPU for PVHVM guest gets its id from CPUID. With secondary
CPUs it is a bit more trickier. Currently, we initialize IPI vectors
before these CPUs boot so we can't use CPUID. However, we know that
physical CPU id (vLAPIC id) is Xen's vCPU id * 2, we can piggyback on
that. Alternatively, we could have disabled all secondary CPUs once we
detect that Xen's and Linux's ideas of vCPU id diverged.

Signed-off-by: Vitaly Kuznetsov 
---
 arch/arm/xen/enlighten.c | 10 ++
 arch/x86/xen/enlighten.c | 18 +-
 include/xen/xen-ops.h|  1 +
 3 files changed, 28 insertions(+), 1 deletion(-)

diff --git a/arch/arm/xen/enlighten.c b/arch/arm/xen/enlighten.c
index 75cd734..ea99ca2 100644
--- a/arch/arm/xen/enlighten.c
+++ b/arch/arm/xen/enlighten.c
@@ -46,6 +46,10 @@ struct shared_info *HYPERVISOR_shared_info = (void 
*)_dummy_shared_info;
 DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
 static struct vcpu_info __percpu *xen_vcpu_info;
 
+/* Linux <-> Xen vCPU id mapping */
+DEFINE_PER_CPU(int, xen_vcpu_id) = -1;
+EXPORT_SYMBOL_GPL(xen_vcpu_id);
+
 /* These are unused until we support booting "pre-ballooned" */
 unsigned long xen_released_pages;
 struct xen_memory_region xen_extra_mem[XEN_EXTRA_MEM_MAX_REGIONS] __initdata;
@@ -179,6 +183,9 @@ static void xen_percpu_init(void)
pr_info("Xen: initializing cpu%d\n", cpu);
vcpup = per_cpu_ptr(xen_vcpu_info, cpu);
 
+   /* Direct vCPU id mapping for ARM guests. */
+   per_cpu(xen_vcpu_id, cpu) = cpu;
+
info.mfn = virt_to_gfn(vcpup);
info.offset = xen_offset_in_page(vcpup);
 
@@ -328,6 +335,9 @@ static int __init xen_guest_init(void)
if (xen_vcpu_info == NULL)
return -ENOMEM;
 
+   /* Direct vCPU id mapping for ARM guests. */
+   per_cpu(xen_vcpu_id, 0) = 0;
+
if (gnttab_setup_auto_xlat_frames(grant_frames)) {
free_percpu(xen_vcpu_info);
return -ENOMEM;
diff --git a/arch/x86/xen/enlighten.c b/arch/x86/xen/enlighten.c
index 760789a..69f4c0c 100644
--- a/arch/x86/xen/enlighten.c
+++ b/arch/x86/xen/enlighten.c
@@ -59,6 +59,7 @@
 #include 
 #include 
 #include 
+#include 
 #include 
 #include 
 #include 
@@ -118,6 +119,10 @@ DEFINE_PER_CPU(struct vcpu_info *, xen_vcpu);
  */
 DEFINE_PER_CPU(struct vcpu_info, xen_vcpu_info);
 
+/* Linux <-> Xen vCPU id mapping */
+DEFINE_PER_CPU(int, xen_vcpu_id) = -1;
+EXPORT_SYMBOL_GPL(xen_vcpu_id);
+
 enum xen_domain_type xen_domain_type = XEN_NATIVE;
 EXPORT_SYMBOL_GPL(xen_domain_type);
 
@@ -1137,8 +1142,11 @@ void xen_setup_vcpu_info_placement(void)
 {
int cpu;
 
-   for_each_possible_cpu(cpu)
+   for_each_possible_cpu(cpu) {
+   /* Set up direct vCPU id mapping for PV guests. */
+   per_cpu(xen_vcpu_id, cpu) = cpu;
xen_vcpu_setup(cpu);
+   }
 
/* xen_vcpu_setup managed to place the vcpu_info within the
 * percpu area for all cpus, so make use of it. Note that for
@@ -1797,6 +1805,12 @@ static void __init init_hvm_pv_info(void)
 
xen_setup_features();
 
+   cpuid(base + 4, , , , );
+   if (eax & XEN_HVM_CPUID_VCPU_ID_PRESENT)
+   this_cpu_write(xen_vcpu_id, ebx);
+   else
+   this_cpu_write(xen_vcpu_id, smp_processor_id());
+
pv_info.name = "Xen HVM";
 
xen_domain_type = XEN_HVM_DOMAIN;
@@ -1808,6 +1822,8 @@ static int xen_hvm_cpu_notify(struct notifier_block 
*self, unsigned long action,
int cpu = (long)hcpu;
switch (action) {
case CPU_UP_PREPARE:
+   /* vLAPIC_ID == Xen's vCPU_ID * 2 for HVM guests */
+   per_cpu(xen_vcpu_id, cpu) = cpu_physical_id(cpu) / 2;
xen_vcpu_setup(cpu);
if (xen_have_vector_callback) {
if (xen_feature(XENFEAT_hvm_safe_pvclock))
diff --git a/include/xen/xen-ops.h b/include/xen/xen-ops.h
index 86abe07..b02a343 100644
--- a/include/xen/xen-ops.h
+++ b/include/xen/xen-ops.h
@@ -8,6 +8,7 @@
 #include 
 
 DECLARE_PER_CPU(struct vcpu_info *, xen_vcpu);
+DECLARE_PER_CPU(int, xen_vcpu_id);
 
 void xen_arch_pre_suspend(void);
 void xen_arch_post_suspend(int suspend_cancelled);
-- 
2.5.5


___
Xen-devel mailing list
Xen-devel@lists.xen.org
http://lists.xen.org/xen-devel