Check that the processor to be woken up APIC ID is addressable in the
current APIC mode.

Note that in practice systems with APIC IDs > 255 should already have
x2APIC enabled by the firmware, and hence this is mostly a safety
belt.

Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
---
Changes since v2:
 - Reword error message.
---
 xen/arch/x86/smpboot.c | 7 +++++++
 1 file changed, 7 insertions(+)

diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
index fa691b6ba0..8cbb7173a4 100644
--- a/xen/arch/x86/smpboot.c
+++ b/xen/arch/x86/smpboot.c
@@ -1317,6 +1317,13 @@ int __cpu_up(unsigned int cpu)
     if ( (apicid = x86_cpu_to_apicid[cpu]) == BAD_APICID )
         return -ENODEV;
 
+    if ( (!x2apic_enabled || !iommu_intremap) && (apicid >> 8) )
+    {
+        printk("Unsupported: APIC ID %#x in xAPIC mode w/o interrupt remapping",
+               apicid);
+        return -EINVAL;
+    }
+
     if ( (ret = do_boot_cpu(apicid, cpu)) != 0 )
         return ret;
 
-- 
2.24.0


_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On 04.12.2019 17:20, Roger Pau Monne wrote:
> Check that the processor to be woken up APIC ID is addressable in the
> current APIC mode.
> 
> Note that in practice systems with APIC IDs > 255 should already have
> x2APIC enabled by the firmware, and hence this is mostly a safety
> belt.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> ---
> Changes since v2:
>  - Reword error message.
> ---
>  xen/arch/x86/smpboot.c | 7 +++++++
>  1 file changed, 7 insertions(+)
> 
> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
> index fa691b6ba0..8cbb7173a4 100644
> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -1317,6 +1317,13 @@ int __cpu_up(unsigned int cpu)
>      if ( (apicid = x86_cpu_to_apicid[cpu]) == BAD_APICID )
>          return -ENODEV;
>  
> +    if ( (!x2apic_enabled || !iommu_intremap) && (apicid >> 8) )

Btw, I'll change the right side to "apicid >= 0xff", as that ID is
special. Or perhaps this should really be

    if ( (!x2apic_enabled && apicid >= APIC_ALL_CPUS) ||
         (!iommu_intremap && (apicid >> 8)) )

Thoughts?

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On 20/12/2019 15:17, Jan Beulich wrote:
> On 04.12.2019 17:20, Roger Pau Monne wrote:
>> Check that the processor to be woken up APIC ID is addressable in the
>> current APIC mode.
>>
>> Note that in practice systems with APIC IDs > 255 should already have
>> x2APIC enabled by the firmware, and hence this is mostly a safety
>> belt.
>>
>> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
>> ---
>> Changes since v2:
>>  - Reword error message.
>> ---
>>  xen/arch/x86/smpboot.c | 7 +++++++
>>  1 file changed, 7 insertions(+)
>>
>> diff --git a/xen/arch/x86/smpboot.c b/xen/arch/x86/smpboot.c
>> index fa691b6ba0..8cbb7173a4 100644
>> --- a/xen/arch/x86/smpboot.c
>> +++ b/xen/arch/x86/smpboot.c
>> @@ -1317,6 +1317,13 @@ int __cpu_up(unsigned int cpu)
>>      if ( (apicid = x86_cpu_to_apicid[cpu]) == BAD_APICID )
>>          return -ENODEV;
>>  
>> +    if ( (!x2apic_enabled || !iommu_intremap) && (apicid >> 8) )
> Btw, I'll change the right side to "apicid >= 0xff", as that ID is
> special. Or perhaps this should really be
>
>     if ( (!x2apic_enabled && apicid >= APIC_ALL_CPUS) ||
>          (!iommu_intremap && (apicid >> 8)) )
>
> Thoughts?

LGTM.

~Andrew

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On 04.12.2019 17:20, Roger Pau Monne wrote:
> Check that the processor to be woken up APIC ID is addressable in the
> current APIC mode.
> 
> Note that in practice systems with APIC IDs > 255 should already have
> x2APIC enabled by the firmware, and hence this is mostly a safety
> belt.
> 
> Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>

Reviewed-by: Jan Beulich <jbeulich@suse.com>
with ...

> --- a/xen/arch/x86/smpboot.c
> +++ b/xen/arch/x86/smpboot.c
> @@ -1317,6 +1317,13 @@ int __cpu_up(unsigned int cpu)
>      if ( (apicid = x86_cpu_to_apicid[cpu]) == BAD_APICID )
>          return -ENODEV;
>  
> +    if ( (!x2apic_enabled || !iommu_intremap) && (apicid >> 8) )
> +    {
> +        printk("Unsupported: APIC ID %#x in xAPIC mode w/o interrupt remapping",
> +               apicid);

... the lost newline added back (can be done while commiting).

Jan

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel

On Thu, Dec 05, 2019 at 10:33:44AM +0100, Jan Beulich wrote:
> On 04.12.2019 17:20, Roger Pau Monne wrote:
> > Check that the processor to be woken up APIC ID is addressable in the
> > current APIC mode.
> > 
> > Note that in practice systems with APIC IDs > 255 should already have
> > x2APIC enabled by the firmware, and hence this is mostly a safety
> > belt.
> > 
> > Signed-off-by: Roger Pau Monné <roger.pau@citrix.com>
> 
> Reviewed-by: Jan Beulich <jbeulich@suse.com>
> with ...
> 
> > --- a/xen/arch/x86/smpboot.c
> > +++ b/xen/arch/x86/smpboot.c
> > @@ -1317,6 +1317,13 @@ int __cpu_up(unsigned int cpu)
> >      if ( (apicid = x86_cpu_to_apicid[cpu]) == BAD_APICID )
> >          return -ENODEV;
> >  
> > +    if ( (!x2apic_enabled || !iommu_intremap) && (apicid >> 8) )
> > +    {
> > +        printk("Unsupported: APIC ID %#x in xAPIC mode w/o interrupt remapping",
> > +               apicid);
> 
> ... the lost newline added back (can be done while commiting).

Sorry for dropping it, please add it at commit.

Thanks, Roger.

_______________________________________________
Xen-devel mailing list
Xen-devel@lists.xenproject.org
https://lists.xenproject.org/mailman/listinfo/xen-devel