No need for an extra variable to find out whether the APIC has been mapped
or is accessible (X2APIC mode).
Provide an inline for this and check apic_mmio_base which is only set when
the local APIC has been mapped.
Signed-off-by: Thomas Gleixner <[email protected]>
---
arch/x86/kernel/apic/apic.c | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
--- a/arch/x86/kernel/apic/apic.c
+++ b/arch/x86/kernel/apic/apic.c
@@ -99,6 +99,11 @@ static bool virt_ext_dest_id __ro_after_
/* For parallel bootup. */
unsigned long apic_mmio_base __ro_after_init;
+static inline bool apic_accessible(void)
+{
+ return x2apic_mode || apic_mmio_base;
+}
+
/*
* Map cpu index to physical APIC ID
*/
@@ -199,8 +204,6 @@ unsigned int lapic_timer_period = 0;
static void apic_pm_activate(void);
-static unsigned long apic_phys __ro_after_init;
-
/*
* Get the LAPIC version
*/
@@ -1127,8 +1130,7 @@ void clear_local_APIC(void)
int maxlvt;
u32 v;
- /* APIC hasn't been mapped yet */
- if (!x2apic_mode && !apic_phys)
+ if (!apic_accessible())
return;
maxlvt = lapic_get_maxlvt();
@@ -1218,8 +1220,7 @@ void apic_soft_disable(void)
*/
void disable_local_APIC(void)
{
- /* APIC hasn't been mapped yet */
- if (!x2apic_mode && !apic_phys)
+ if (!apic_accessible())
return;
apic_soft_disable();
@@ -1921,7 +1922,6 @@ static __init void try_to_enable_x2apic(
* be addressed must not be brought online.
*/
x2apic_set_max_apicid(apic_limit);
- x2apic_phys = 1;
}
x2apic_enable();
}
@@ -2895,11 +2895,11 @@ early_param("apic", apic_set_verbosity);
static int __init lapic_insert_resource(void)
{
- if (!apic_phys)
+ if (!apic_mmio_base)
return -1;
/* Put local APIC into the resource map. */
- lapic_resource.start = apic_phys;
+ lapic_resource.start = apic_mmio_base;
lapic_resource.end = lapic_resource.start + PAGE_SIZE - 1;
insert_resource(&iomem_resource, &lapic_resource);
On Tue, Jul 18 2023 at 01:15, Thomas Gleixner wrote:
> @@ -1921,7 +1922,6 @@ static __init void try_to_enable_x2apic(
> * be addressed must not be brought online.
> */
> x2apic_set_max_apicid(apic_limit);
> - x2apic_phys = 1;
> }
> x2apic_enable();
> }
This hunk is obviously bogus. I just noticed on a VM which takes this
code path...
From: Thomas Gleixner <[email protected]> Sent: Tuesday, July 18, 2023 6:12 AM
>
> On Tue, Jul 18 2023 at 01:15, Thomas Gleixner wrote:
> > @@ -1921,7 +1922,6 @@ static __init void try_to_enable_x2apic(
> > * be addressed must not be brought online.
> > */
> > x2apic_set_max_apicid(apic_limit);
> > - x2apic_phys = 1;
> > }
> > x2apic_enable();
> > }
>
> This hunk is obviously bogus. I just noticed on a VM which takes this
> code path...
I'm testing guests on Hyper-V. The case where the x2apic is enabled
in the BIOS works, but when the x2apic must be enabled by Linux,
the VMbus drivers never get initialized and things go downhill from
there. Your comment above is somewhat cryptic (as I haven't studied
the patches in detail), but I'm guessing it explains the failure I'm seeing.
Let me know if I should debug the failure I'm seeing. Otherwise
I'll wait for a new version and try again.
Michael
On Thu, Jul 20 2023 at 04:18, Michael Kelley wrote:
> From: Thomas Gleixner <[email protected]> Sent: Tuesday, July 18, 2023 6:12 AM
>>
>> On Tue, Jul 18 2023 at 01:15, Thomas Gleixner wrote:
>> > @@ -1921,7 +1922,6 @@ static __init void try_to_enable_x2apic(
>> > * be addressed must not be brought online.
>> > */
>> > x2apic_set_max_apicid(apic_limit);
>> > - x2apic_phys = 1;
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This hunk _IS_ wrong and needs to be
reverted obvioulsy.
>> > }
>> > x2apic_enable();
>> > }
>>
>> This hunk is obviously bogus. I just noticed on a VM which takes this
>> code path...
>
> I'm testing guests on Hyper-V. The case where the x2apic is enabled
> in the BIOS works, but when the x2apic must be enabled by Linux,
> the VMbus drivers never get initialized and things go downhill from
> there. Your comment above is somewhat cryptic (as I haven't studied
> the patches in detail), but I'm guessing it explains the failure I'm seeing.
>
> Let me know if I should debug the failure I'm seeing. Otherwise
> I'll wait for a new version and try again.
Can you add that line back and retest?
Thanks,
tglx
From: Thomas Gleixner <[email protected]> Sent: Thursday, July 20, 2023 1:03 AM
>
> On Thu, Jul 20 2023 at 04:18, Michael Kelley wrote:
>
> > From: Thomas Gleixner <[email protected]> Sent: Tuesday, July 18, 2023 6:12 AM
> >>
> >> On Tue, Jul 18 2023 at 01:15, Thomas Gleixner wrote:
> >> > @@ -1921,7 +1922,6 @@ static __init void try_to_enable_x2apic(
> >> > * be addressed must not be brought online.
> >> > */
> >> > x2apic_set_max_apicid(apic_limit);
> >> > - x2apic_phys = 1;
>
> ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ This hunk _IS_ wrong and needs to be
> reverted obvioulsy.
>
> >> > }
> >> > x2apic_enable();
> >> > }
> >>
> >> This hunk is obviously bogus. I just noticed on a VM which takes this
> >> code path...
> >
> > I'm testing guests on Hyper-V. The case where the x2apic is enabled
> > in the BIOS works, but when the x2apic must be enabled by Linux,
> > the VMbus drivers never get initialized and things go downhill from
> > there. Your comment above is somewhat cryptic (as I haven't studied
> > the patches in detail), but I'm guessing it explains the failure I'm seeing.
> >
> > Let me know if I should debug the failure I'm seeing. Otherwise
> > I'll wait for a new version and try again.
>
> Can you add that line back and retest?
>
Everything is looking good now -- my VMbus driver problem was actually
an unrelated config problem. All combinations of Hyper-V VMs that
I've tried with xapic and x2apic, and on Intel and AMD processors,
are working. There are couple more old configs that I want to try
tomorrow or over the weekend.
Michael
Michael