| Message ID | 20231102-x86-apic-v1-0-bf049a2a0ed6@citrix.com (mailing list archive) |
|---|---|
| Headers | show |
| Series | x86/apic: Misc pruning | expand |
On 02/11/2023 12:26 pm, Andrew Cooper wrote: > Seriously, this work started out trying to fix a buggy comment. It > escalated somewhat... Perform some simple tidying. Another dodgy construct spotted while doing this work is #ifdef CONFIG_X86_32 #define BAD_APICID 0xFFu #else #define BAD_APICID 0xFFFFu #endif considering that both of those "bad" values are legal APIC IDs in an x2APIC system. The majority use is as a sentential (of varying types - int, u16 mostly), although the uses for NUM_APIC_CLUSTERS, and safe_smp_processor_id() look suspect. In particular, safe_smp_processor_id() *will* malfunction on some legal CPUs, and needs to use -1 (32 bits wide) to spot the intended error case of a bad xAPIC mapping. However, it's use in amd_pmu_cpu_starting() from topology_die_id() looks broken. Partly because the error handling is (only) a WARN_ON_ONCE(), and also because nb->nb_id's sentinel value is -1 of type int. I suspect there's a lot of cleaning to be done here too. ~Andrew
On Fri, Nov 03 2023 at 19:58, Andrew Cooper wrote: > On 02/11/2023 12:26 pm, Andrew Cooper wrote: > Another dodgy construct spotted while doing this work is > > #ifdef CONFIG_X86_32 > #define BAD_APICID 0xFFu > #else > #define BAD_APICID 0xFFFFu > #endif > > considering that both of those "bad" values are legal APIC IDs in an > x2APIC system. > > The majority use is as a sentential (of varying types - int, u16 > mostly), although the uses for NUM_APIC_CLUSTERS, and > safe_smp_processor_id() look suspect. > > In particular, safe_smp_processor_id() *will* malfunction on some legal > CPUs, and needs to use -1 (32 bits wide) to spot the intended error case > of a bad xAPIC mapping. > > However, it's use in amd_pmu_cpu_starting() from topology_die_id() looks > broken. Partly because the error handling is (only) a WARN_ON_ONCE(), > and also because nb->nb_id's sentinel value is -1 of type int. > > I suspect there's a lot of cleaning to be done here too. Sigh...
Seriously, this work started out trying to fix a buggy comment. It escalated somewhat... Perform some simple tidying. P.S. I'm trialing `b4 prep` to send this series. I've got some notes already; others welcome too. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- Andrew Cooper (3): x86/apic: Drop apic::delivery_mode x86/apic: Drop enum apic_delivery_modes x86/apic: Drop struct local_apic arch/x86/include/asm/apic.h | 2 - arch/x86/include/asm/apicdef.h | 276 +--------------------------------- arch/x86/kernel/apic/apic_flat_64.c | 2 - arch/x86/kernel/apic/apic_noop.c | 1 - arch/x86/kernel/apic/apic_numachip.c | 2 - arch/x86/kernel/apic/bigsmp_32.c | 1 - arch/x86/kernel/apic/probe_32.c | 1 - arch/x86/kernel/apic/x2apic_cluster.c | 1 - arch/x86/kernel/apic/x2apic_phys.c | 1 - arch/x86/kernel/apic/x2apic_uv_x.c | 1 - arch/x86/platform/uv/uv_irq.c | 2 +- drivers/pci/controller/pci-hyperv.c | 7 - 12 files changed, 8 insertions(+), 289 deletions(-) --- base-commit: b56ebe7c896dc78b5865ec2c4b1dae3c93537517 change-id: 20231102-x86-apic-88dc3eae3032 Best regards,