diff mbox

[RFC,V2,2/4] Tool/ACPI: DSDT extension to support more vcpus

Message ID 1504155709-24276-3-git-send-email-tianyu.lan@intel.com (mailing list archive)
State New, archived
Headers show

Commit Message

lan,Tianyu Aug. 31, 2017, 5:01 a.m. UTC
This patch is to change DSDT table for processor object to support >128 vcpus
accroding to ACPI spec 8.4 Declaring Processors

Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
---
 tools/libacpi/mk_dsdt.c | 18 ++++++++++++------
 1 file changed, 12 insertions(+), 6 deletions(-)

Comments

Roger Pau Monné Aug. 31, 2017, 3:38 p.m. UTC | #1
On Thu, Aug 31, 2017 at 01:01:47AM -0400, Lan Tianyu wrote:
> This patch is to change DSDT table for processor object to support >128 vcpus
> accroding to ACPI spec 8.4 Declaring Processors
> 
> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> ---
>  tools/libacpi/mk_dsdt.c | 18 ++++++++++++------
>  1 file changed, 12 insertions(+), 6 deletions(-)
> 
> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
> index 2daf32c..6c4c325 100644
> --- a/tools/libacpi/mk_dsdt.c
> +++ b/tools/libacpi/mk_dsdt.c
> @@ -24,6 +24,8 @@
>  #include <xen/arch-arm.h>
>  #endif
>  
> +#define CPU_NAME_FMT      "P%.03X"
> +
>  static unsigned int indent_level;
>  static bool debug = false;
>  
> @@ -196,10 +198,14 @@ int main(int argc, char **argv)
>      /* Define processor objects and control methods. */
>      for ( cpu = 0; cpu < max_cpus; cpu++)
>      {
> -        push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu, cpu);
> +        unsigned int apic_id = cpu * 2;

This is fragile, ideally there should be a single point where the APIC
ID is calculated. Although there are already two places where the APIC
ID is calculated, in hvmloader and libxl.

And I'm not sure how to use any of those here in order to avoid
introducing a third one.

>  
> -        stmt("Name", "_HID, \"ACPI0007\"");
> +        if ( apic_id > 255 )

We need to be careful with this. This is not a problem ATM because the
ACPI ID is the CPU ID, but care should be taken to not create a
Processor object with ACPI ID 255, because that's the broadcast ACPI
ID...

> +            push_block("Device", CPU_NAME_FMT, cpu);
> +        else

... IMHO an assert(cpu < 255); should be added here.

> +            push_block("Processor", CPU_NAME_FMT", %d, 0x0000b010, 0x06", cpu, cpu);
                                                   ^ space (here and below)

Please leave a space between the string literals and the defines, it
makes it easier to read. And this line needs to be split.

>  
> +        stmt("Name", "_HID, \"ACPI0007\"");
>          stmt("Name", "_UID, %d", cpu);
>  #ifdef CONFIG_ARM_64
>          pop_block();
> @@ -268,15 +274,15 @@ int main(int argc, char **argv)
>          /* Extract current CPU's status: 0=offline; 1=online. */
>          stmt("And", "Local1, 1, Local2");
>          /* Check if status is up-to-date in the relevant MADT LAPIC entry... */
> -        push_block("If", "LNotEqual(Local2, \\_SB.PR%02X.FLG)", cpu);
> +        push_block("If", "LNotEqual(Local2, \\_SB."CPU_NAME_FMT ".FLG)", cpu);
>          /* ...If not, update it and the MADT checksum, and notify OSPM. */
> -        stmt("Store", "Local2, \\_SB.PR%02X.FLG", cpu);
> +        stmt("Store", "Local2, \\_SB."CPU_NAME_FMT".FLG", cpu);
>          push_block("If", "LEqual(Local2, 1)");
> -        stmt("Notify", "PR%02X, 1", cpu); /* Notify: Device Check */
> +        stmt("Notify", CPU_NAME_FMT", 1", cpu); /* Notify: Device Check */
>          stmt("Subtract", "\\_SB.MSU, 1, \\_SB.MSU"); /* Adjust MADT csum */
>          pop_block();
>          push_block("Else", NULL);
> -        stmt("Notify", "PR%02X, 3", cpu); /* Notify: Eject Request */
> +        stmt("Notify", CPU_NAME_FMT", 3", cpu); /* Notify: Eject Request */
>          stmt("Add", "\\_SB.MSU, 1, \\_SB.MSU"); /* Adjust MADT csum */
>          pop_block();
>          pop_block();
> -- 
> 1.8.3.1
>
lan,Tianyu Sept. 1, 2017, 2:54 a.m. UTC | #2
On 2017年08月31日 23:38, Roger Pau Monné wrote:
> On Thu, Aug 31, 2017 at 01:01:47AM -0400, Lan Tianyu wrote:
>> This patch is to change DSDT table for processor object to support >128 vcpus
>> accroding to ACPI spec 8.4 Declaring Processors
>>
>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>> ---
>>  tools/libacpi/mk_dsdt.c | 18 ++++++++++++------
>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>
>> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
>> index 2daf32c..6c4c325 100644
>> --- a/tools/libacpi/mk_dsdt.c
>> +++ b/tools/libacpi/mk_dsdt.c
>> @@ -24,6 +24,8 @@
>>  #include <xen/arch-arm.h>
>>  #endif
>>  
>> +#define CPU_NAME_FMT      "P%.03X"
>> +
>>  static unsigned int indent_level;
>>  static bool debug = false;
>>  
>> @@ -196,10 +198,14 @@ int main(int argc, char **argv)
>>      /* Define processor objects and control methods. */
>>      for ( cpu = 0; cpu < max_cpus; cpu++)
>>      {
>> -        push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu, cpu);
>> +        unsigned int apic_id = cpu * 2;
> 
> This is fragile, ideally there should be a single point where the APIC
> ID is calculated. Although there are already two places where the APIC
> ID is calculated, in hvmloader and libxl.
> 
> And I'm not sure how to use any of those here in order to avoid
> introducing a third one.

The mk_dsdt is independent tool to build dsdt table. It wasn't linked
with libxl and hvmloader. We can't reuse old function to do that.

But I think we may introduce a new LAPIC_ID(vcpu) in the arch head
file(i.e, #include <xen/arch-x86/xen.h>) and replace old ones.

> 
>>  
>> -        stmt("Name", "_HID, \"ACPI0007\"");
>> +        if ( apic_id > 255 )
> 
> We need to be careful with this. This is not a problem ATM because the
> ACPI ID is the CPU ID, but care should be taken to not create a
> Processor object with ACPI ID 255, because that's the broadcast ACPI
> ID...

Yes.

> 
>> +            push_block("Device", CPU_NAME_FMT, cpu);
>> +        else
> 
> ... IMHO an assert(cpu < 255); should be added here.

OK.

> 
>> +            push_block("Processor", CPU_NAME_FMT", %d, 0x0000b010, 0x06", cpu, cpu);
>                                                    ^ space (here and below)
> 
> Please leave a space between the string literals and the defines, it
> makes it easier to read. And this line needs to be split.
> 

OK. Will update.
Roger Pau Monné Sept. 1, 2017, 9:41 a.m. UTC | #3
On Fri, Sep 01, 2017 at 10:54:02AM +0800, Lan Tianyu wrote:
> On 2017年08月31日 23:38, Roger Pau Monné wrote:
> > On Thu, Aug 31, 2017 at 01:01:47AM -0400, Lan Tianyu wrote:
> >> This patch is to change DSDT table for processor object to support >128 vcpus
> >> accroding to ACPI spec 8.4 Declaring Processors
> >>
> >> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> >> ---
> >>  tools/libacpi/mk_dsdt.c | 18 ++++++++++++------
> >>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>
> >> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
> >> index 2daf32c..6c4c325 100644
> >> --- a/tools/libacpi/mk_dsdt.c
> >> +++ b/tools/libacpi/mk_dsdt.c
> >> @@ -24,6 +24,8 @@
> >>  #include <xen/arch-arm.h>
> >>  #endif
> >>  
> >> +#define CPU_NAME_FMT      "P%.03X"
> >> +
> >>  static unsigned int indent_level;
> >>  static bool debug = false;
> >>  
> >> @@ -196,10 +198,14 @@ int main(int argc, char **argv)
> >>      /* Define processor objects and control methods. */
> >>      for ( cpu = 0; cpu < max_cpus; cpu++)
> >>      {
> >> -        push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu, cpu);
> >> +        unsigned int apic_id = cpu * 2;
> > 
> > This is fragile, ideally there should be a single point where the APIC
> > ID is calculated. Although there are already two places where the APIC
> > ID is calculated, in hvmloader and libxl.
> > 
> > And I'm not sure how to use any of those here in order to avoid
> > introducing a third one.
> 
> The mk_dsdt is independent tool to build dsdt table. It wasn't linked
> with libxl and hvmloader. We can't reuse old function to do that.
> 
> But I think we may introduce a new LAPIC_ID(vcpu) in the arch head
> file(i.e, #include <xen/arch-x86/xen.h>) and replace old ones.

There's already a LAPIC_ID macro in hvmloader headers which should be
placed somewhere suitable. What about removing the lapic_id hook from
acpi_config and placing the LAPIC_ID macro in the libacpi.h header?

I'm not sure why lapic_id needs to be a hook in any case, both it's
callers use the same exact formula (cpu_id * 2).

Thanks, Roger.
Jan Beulich Sept. 1, 2017, 10:10 a.m. UTC | #4
>>> On 01.09.17 at 11:41, <roger.pau@citrix.com> wrote:
> I'm not sure why lapic_id needs to be a hook in any case, both it's
> callers use the same exact formula (cpu_id * 2).

You're aware that this is meant to be improved rather sooner than
later. At that point I'm then not sure a suitable LAPIC_ID() macro
could be created, as the potential users of this may not have
available all necessary inputs. I'd strongly suggest finding a way
to dynamically insert the APIC IDs (as found while booting) into
the ACPI tables. That's pretty likely how real BIOSes have to do
it too.

Jan
Roger Pau Monné Sept. 1, 2017, 10:26 a.m. UTC | #5
On Fri, Sep 01, 2017 at 04:10:26AM -0600, Jan Beulich wrote:
> >>> On 01.09.17 at 11:41, <roger.pau@citrix.com> wrote:
> > I'm not sure why lapic_id needs to be a hook in any case, both it's
> > callers use the same exact formula (cpu_id * 2).
> 
> You're aware that this is meant to be improved rather sooner than
> later. At that point I'm then not sure a suitable LAPIC_ID() macro
> could be created, as the potential users of this may not have
> available all necessary inputs. I'd strongly suggest finding a way
> to dynamically insert the APIC IDs (as found while booting) into
> the ACPI tables. That's pretty likely how real BIOSes have to do
> it too.

There's a further problem here, which is that the DSDT is created at
tools compilation time, there's no way to know the APIC IDs at that
point in order to decide whether a CPU gets a Processor or a Device
object.

Leaving the DSDT issue aside, there's also the problem that the MADT
for PVH guests is build by the toolstack, not the firmware (because
there's none), so it's impossible to do it like real BIOSes.

So AFAICT we either remove the hook and code the logic to get the ACPI
ID inside libacpi, or we introduce yet another open coded one in order
to generate the DSDT from mk_dsdt.c

Roger.
lan,Tianyu Sept. 4, 2017, 3:07 a.m. UTC | #6
On 2017年09月01日 17:41, Roger Pau Monné wrote:
> On Fri, Sep 01, 2017 at 10:54:02AM +0800, Lan Tianyu wrote:
>> On 2017年08月31日 23:38, Roger Pau Monné wrote:
>>> On Thu, Aug 31, 2017 at 01:01:47AM -0400, Lan Tianyu wrote:
>>>> This patch is to change DSDT table for processor object to support >128 vcpus
>>>> accroding to ACPI spec 8.4 Declaring Processors
>>>>
>>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>>>> ---
>>>>  tools/libacpi/mk_dsdt.c | 18 ++++++++++++------
>>>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
>>>> index 2daf32c..6c4c325 100644
>>>> --- a/tools/libacpi/mk_dsdt.c
>>>> +++ b/tools/libacpi/mk_dsdt.c
>>>> @@ -24,6 +24,8 @@
>>>>  #include <xen/arch-arm.h>
>>>>  #endif
>>>>  
>>>> +#define CPU_NAME_FMT      "P%.03X"
>>>> +
>>>>  static unsigned int indent_level;
>>>>  static bool debug = false;
>>>>  
>>>> @@ -196,10 +198,14 @@ int main(int argc, char **argv)
>>>>      /* Define processor objects and control methods. */
>>>>      for ( cpu = 0; cpu < max_cpus; cpu++)
>>>>      {
>>>> -        push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu, cpu);
>>>> +        unsigned int apic_id = cpu * 2;
>>>
>>> This is fragile, ideally there should be a single point where the APIC
>>> ID is calculated. Although there are already two places where the APIC
>>> ID is calculated, in hvmloader and libxl.
>>>
>>> And I'm not sure how to use any of those here in order to avoid
>>> introducing a third one.
>>
>> The mk_dsdt is independent tool to build dsdt table. It wasn't linked
>> with libxl and hvmloader. We can't reuse old function to do that.
>>
>> But I think we may introduce a new LAPIC_ID(vcpu) in the arch head
>> file(i.e, #include <xen/arch-x86/xen.h>) and replace old ones.
> 
> There's already a LAPIC_ID macro in hvmloader headers which should be
> placed somewhere suitable.

Yes, this is what I mentioned.

> What about removing the lapic_id hook from
> acpi_config and placing the LAPIC_ID macro in the libacpi.h header?

I think this should be ARCH specific. I am not sure whether ARM follows
rule of "apic_id = vcpu_id *2".

Julien, could you give some inputs? Thanks.



> 
> I'm not sure why lapic_id needs to be a hook in any case, both it's
> callers use the same exact formula (cpu_id * 2).
> 
> Thanks, Roger.
>
Roger Pau Monné Sept. 4, 2017, 9:05 a.m. UTC | #7
On Mon, Sep 04, 2017 at 11:07:14AM +0800, Lan Tianyu wrote:
> On 2017年09月01日 17:41, Roger Pau Monné wrote:
> > On Fri, Sep 01, 2017 at 10:54:02AM +0800, Lan Tianyu wrote:
> >> On 2017年08月31日 23:38, Roger Pau Monné wrote:
> >>> On Thu, Aug 31, 2017 at 01:01:47AM -0400, Lan Tianyu wrote:
> >>>> This patch is to change DSDT table for processor object to support >128 vcpus
> >>>> accroding to ACPI spec 8.4 Declaring Processors
> >>>>
> >>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
> >>>> ---
> >>>>  tools/libacpi/mk_dsdt.c | 18 ++++++++++++------
> >>>>  1 file changed, 12 insertions(+), 6 deletions(-)
> >>>>
> >>>> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
> >>>> index 2daf32c..6c4c325 100644
> >>>> --- a/tools/libacpi/mk_dsdt.c
> >>>> +++ b/tools/libacpi/mk_dsdt.c
> >>>> @@ -24,6 +24,8 @@
> >>>>  #include <xen/arch-arm.h>
> >>>>  #endif
> >>>>  
> >>>> +#define CPU_NAME_FMT      "P%.03X"
> >>>> +
> >>>>  static unsigned int indent_level;
> >>>>  static bool debug = false;
> >>>>  
> >>>> @@ -196,10 +198,14 @@ int main(int argc, char **argv)
> >>>>      /* Define processor objects and control methods. */
> >>>>      for ( cpu = 0; cpu < max_cpus; cpu++)
> >>>>      {
> >>>> -        push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu, cpu);
> >>>> +        unsigned int apic_id = cpu * 2;
> >>>
> >>> This is fragile, ideally there should be a single point where the APIC
> >>> ID is calculated. Although there are already two places where the APIC
> >>> ID is calculated, in hvmloader and libxl.
> >>>
> >>> And I'm not sure how to use any of those here in order to avoid
> >>> introducing a third one.
> >>
> >> The mk_dsdt is independent tool to build dsdt table. It wasn't linked
> >> with libxl and hvmloader. We can't reuse old function to do that.
> >>
> >> But I think we may introduce a new LAPIC_ID(vcpu) in the arch head
> >> file(i.e, #include <xen/arch-x86/xen.h>) and replace old ones.
> > 
> > There's already a LAPIC_ID macro in hvmloader headers which should be
> > placed somewhere suitable.
> 
> Yes, this is what I mentioned.

Jan has expressed some concerns with removing the hook, see:

<59A94E320200007800176754@prv-mh.provo.novell.com>

> > What about removing the lapic_id hook from
> > acpi_config and placing the LAPIC_ID macro in the libacpi.h header?
> 
> I think this should be ARCH specific. I am not sure whether ARM follows
> rule of "apic_id = vcpu_id *2".
> 
> Julien, could you give some inputs? Thanks.

AFAIK ARM doesn't have a local APIC, so there are no xAPIC/x2APIC
entries in the ARM MADT.

Roger.
lan,Tianyu Sept. 4, 2017, 11:16 a.m. UTC | #8
On 2017年09月04日 17:05, Roger Pau Monné wrote:
> On Mon, Sep 04, 2017 at 11:07:14AM +0800, Lan Tianyu wrote:
>> On 2017年09月01日 17:41, Roger Pau Monné wrote:
>>> On Fri, Sep 01, 2017 at 10:54:02AM +0800, Lan Tianyu wrote:
>>>> On 2017年08月31日 23:38, Roger Pau Monné wrote:
>>>>> On Thu, Aug 31, 2017 at 01:01:47AM -0400, Lan Tianyu wrote:
>>>>>> This patch is to change DSDT table for processor object to support >128 vcpus
>>>>>> accroding to ACPI spec 8.4 Declaring Processors
>>>>>>
>>>>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>>>>>> ---
>>>>>>  tools/libacpi/mk_dsdt.c | 18 ++++++++++++------
>>>>>>  1 file changed, 12 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
>>>>>> index 2daf32c..6c4c325 100644
>>>>>> --- a/tools/libacpi/mk_dsdt.c
>>>>>> +++ b/tools/libacpi/mk_dsdt.c
>>>>>> @@ -24,6 +24,8 @@
>>>>>>  #include <xen/arch-arm.h>
>>>>>>  #endif
>>>>>>  
>>>>>> +#define CPU_NAME_FMT      "P%.03X"
>>>>>> +
>>>>>>  static unsigned int indent_level;
>>>>>>  static bool debug = false;
>>>>>>  
>>>>>> @@ -196,10 +198,14 @@ int main(int argc, char **argv)
>>>>>>      /* Define processor objects and control methods. */
>>>>>>      for ( cpu = 0; cpu < max_cpus; cpu++)
>>>>>>      {
>>>>>> -        push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu, cpu);
>>>>>> +        unsigned int apic_id = cpu * 2;
>>>>>
>>>>> This is fragile, ideally there should be a single point where the APIC
>>>>> ID is calculated. Although there are already two places where the APIC
>>>>> ID is calculated, in hvmloader and libxl.
>>>>>
>>>>> And I'm not sure how to use any of those here in order to avoid
>>>>> introducing a third one.
>>>>
>>>> The mk_dsdt is independent tool to build dsdt table. It wasn't linked
>>>> with libxl and hvmloader. We can't reuse old function to do that.
>>>>
>>>> But I think we may introduce a new LAPIC_ID(vcpu) in the arch head
>>>> file(i.e, #include <xen/arch-x86/xen.h>) and replace old ones.
>>>
>>> There's already a LAPIC_ID macro in hvmloader headers which should be
>>> placed somewhere suitable.
>>
>> Yes, this is what I mentioned.
> 
> Jan has expressed some concerns with removing the hook, see:
> 
> <59A94E320200007800176754@prv-mh.provo.novell.com>

So we still need to introduce LAPIC_ID() here, right?

> 
>>> What about removing the lapic_id hook from
>>> acpi_config and placing the LAPIC_ID macro in the libacpi.h header?
>>
>> I think this should be ARCH specific. I am not sure whether ARM follows
>> rule of "apic_id = vcpu_id *2".
>>
>> Julien, could you give some inputs? Thanks.
> 
> AFAIK ARM doesn't have a local APIC, so there are no xAPIC/x2APIC
> entries in the ARM MADT.
>
Julien Grall Sept. 11, 2017, 11:15 a.m. UTC | #9
Hi,

Sorry for the late reply.

On 04/09/17 10:05, Roger Pau Monné wrote:
> On Mon, Sep 04, 2017 at 11:07:14AM +0800, Lan Tianyu wrote:
>> On 2017年09月01日 17:41, Roger Pau Monné wrote:
>>> On Fri, Sep 01, 2017 at 10:54:02AM +0800, Lan Tianyu wrote:
>>>> On 2017年08月31日 23:38, Roger Pau Monné wrote:
>>>>> On Thu, Aug 31, 2017 at 01:01:47AM -0400, Lan Tianyu wrote:
>>>>>> This patch is to change DSDT table for processor object to support >128 vcpus
>>>>>> accroding to ACPI spec 8.4 Declaring Processors
>>>>>>
>>>>>> Signed-off-by: Lan Tianyu <tianyu.lan@intel.com>
>>>>>> ---
>>>>>>   tools/libacpi/mk_dsdt.c | 18 ++++++++++++------
>>>>>>   1 file changed, 12 insertions(+), 6 deletions(-)
>>>>>>
>>>>>> diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
>>>>>> index 2daf32c..6c4c325 100644
>>>>>> --- a/tools/libacpi/mk_dsdt.c
>>>>>> +++ b/tools/libacpi/mk_dsdt.c
>>>>>> @@ -24,6 +24,8 @@
>>>>>>   #include <xen/arch-arm.h>
>>>>>>   #endif
>>>>>>   
>>>>>> +#define CPU_NAME_FMT      "P%.03X"
>>>>>> +
>>>>>>   static unsigned int indent_level;
>>>>>>   static bool debug = false;
>>>>>>   
>>>>>> @@ -196,10 +198,14 @@ int main(int argc, char **argv)
>>>>>>       /* Define processor objects and control methods. */
>>>>>>       for ( cpu = 0; cpu < max_cpus; cpu++)
>>>>>>       {
>>>>>> -        push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu, cpu);
>>>>>> +        unsigned int apic_id = cpu * 2;
>>>>>
>>>>> This is fragile, ideally there should be a single point where the APIC
>>>>> ID is calculated. Although there are already two places where the APIC
>>>>> ID is calculated, in hvmloader and libxl.
>>>>>
>>>>> And I'm not sure how to use any of those here in order to avoid
>>>>> introducing a third one.
>>>>
>>>> The mk_dsdt is independent tool to build dsdt table. It wasn't linked
>>>> with libxl and hvmloader. We can't reuse old function to do that.
>>>>
>>>> But I think we may introduce a new LAPIC_ID(vcpu) in the arch head
>>>> file(i.e, #include <xen/arch-x86/xen.h>) and replace old ones.
>>>
>>> There's already a LAPIC_ID macro in hvmloader headers which should be
>>> placed somewhere suitable.
>>
>> Yes, this is what I mentioned.
> 
> Jan has expressed some concerns with removing the hook, see:
> 
> <59A94E320200007800176754@prv-mh.provo.novell.com>
> 
>>> What about removing the lapic_id hook from
>>> acpi_config and placing the LAPIC_ID macro in the libacpi.h header?
>>
>> I think this should be ARCH specific. I am not sure whether ARM follows
>> rule of "apic_id = vcpu_id *2".
>>
>> Julien, could you give some inputs? Thanks.
> 
> AFAIK ARM doesn't have a local APIC, so there are no xAPIC/x2APIC
> entries in the ARM MADT.

That's correct. ARM does not use xAPIC/x2APIC tables.

Cheers,
diff mbox

Patch

diff --git a/tools/libacpi/mk_dsdt.c b/tools/libacpi/mk_dsdt.c
index 2daf32c..6c4c325 100644
--- a/tools/libacpi/mk_dsdt.c
+++ b/tools/libacpi/mk_dsdt.c
@@ -24,6 +24,8 @@ 
 #include <xen/arch-arm.h>
 #endif
 
+#define CPU_NAME_FMT      "P%.03X"
+
 static unsigned int indent_level;
 static bool debug = false;
 
@@ -196,10 +198,14 @@  int main(int argc, char **argv)
     /* Define processor objects and control methods. */
     for ( cpu = 0; cpu < max_cpus; cpu++)
     {
-        push_block("Processor", "PR%02X, %d, 0x0000b010, 0x06", cpu, cpu);
+        unsigned int apic_id = cpu * 2;
 
-        stmt("Name", "_HID, \"ACPI0007\"");
+        if ( apic_id > 255 )
+            push_block("Device", CPU_NAME_FMT, cpu);
+        else
+            push_block("Processor", CPU_NAME_FMT", %d, 0x0000b010, 0x06", cpu, cpu);
 
+        stmt("Name", "_HID, \"ACPI0007\"");
         stmt("Name", "_UID, %d", cpu);
 #ifdef CONFIG_ARM_64
         pop_block();
@@ -268,15 +274,15 @@  int main(int argc, char **argv)
         /* Extract current CPU's status: 0=offline; 1=online. */
         stmt("And", "Local1, 1, Local2");
         /* Check if status is up-to-date in the relevant MADT LAPIC entry... */
-        push_block("If", "LNotEqual(Local2, \\_SB.PR%02X.FLG)", cpu);
+        push_block("If", "LNotEqual(Local2, \\_SB."CPU_NAME_FMT ".FLG)", cpu);
         /* ...If not, update it and the MADT checksum, and notify OSPM. */
-        stmt("Store", "Local2, \\_SB.PR%02X.FLG", cpu);
+        stmt("Store", "Local2, \\_SB."CPU_NAME_FMT".FLG", cpu);
         push_block("If", "LEqual(Local2, 1)");
-        stmt("Notify", "PR%02X, 1", cpu); /* Notify: Device Check */
+        stmt("Notify", CPU_NAME_FMT", 1", cpu); /* Notify: Device Check */
         stmt("Subtract", "\\_SB.MSU, 1, \\_SB.MSU"); /* Adjust MADT csum */
         pop_block();
         push_block("Else", NULL);
-        stmt("Notify", "PR%02X, 3", cpu); /* Notify: Eject Request */
+        stmt("Notify", CPU_NAME_FMT", 3", cpu); /* Notify: Eject Request */
         stmt("Add", "\\_SB.MSU, 1, \\_SB.MSU"); /* Adjust MADT csum */
         pop_block();
         pop_block();