| Message ID | 20231102-x86-apic-v1-1-bf049a2a0ed6@citrix.com (mailing list archive) |
|---|---|
| State | Handled Elsewhere |
| Headers | show |
| Series | x86/apic: Misc pruning | expand |
On Thu, Nov 02, 2023 at 12:26:19PM +0000, Andrew Cooper wrote: > This field is set to APIC_DELIVERY_MODE_FIXED in all cases, and is read > exactly once. Fold the constant in uv_program_mmr() and drop the field. > > Searching for the origin of the stale HyperV comment reveals commit > a31e58e129f7 ("x86/apic: Switch all APICs to Fixed delivery mode") which > notes: > > As a consequence of this change, the apic::irq_delivery_mode field is > now pointless, but this needs to be cleaned up in a separate patch. > > 6 years is long enough for this technical debt to have survived. > > Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> Reveiewed-by: Steve Wahl <steve.wahl@hpe.com> > --- > arch/x86/include/asm/apic.h | 2 -- > 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 ------- > 11 files changed, 1 insertion(+), 20 deletions(-) > > diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h > index 5af4ec1a0f71..841afbd7bfe7 100644 > --- a/arch/x86/include/asm/apic.h > +++ b/arch/x86/include/asm/apic.h > @@ -272,8 +272,6 @@ struct apic { > void (*send_IPI_all)(int vector); > void (*send_IPI_self)(int vector); > > - enum apic_delivery_modes delivery_mode; > - > u32 disable_esr : 1, > dest_mode_logical : 1, > x2apic_set_max_apicid : 1; > diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c > index 032a84e2c3cc..e526b226910b 100644 > --- a/arch/x86/kernel/apic/apic_flat_64.c > +++ b/arch/x86/kernel/apic/apic_flat_64.c > @@ -82,7 +82,6 @@ static struct apic apic_flat __ro_after_init = { > .acpi_madt_oem_check = flat_acpi_madt_oem_check, > .apic_id_registered = default_apic_id_registered, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = true, > > .disable_esr = 0, > @@ -153,7 +152,6 @@ static struct apic apic_physflat __ro_after_init = { > .acpi_madt_oem_check = physflat_acpi_madt_oem_check, > .apic_id_registered = default_apic_id_registered, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = false, > > .disable_esr = 0, > diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c > index 966d7cf10b95..70e7dfc3cc84 100644 > --- a/arch/x86/kernel/apic/apic_noop.c > +++ b/arch/x86/kernel/apic/apic_noop.c > @@ -45,7 +45,6 @@ static void noop_apic_write(u32 reg, u32 val) > struct apic apic_noop __ro_after_init = { > .name = "noop", > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = true, > > .disable_esr = 0, > diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c > index 63f3d7be9dc7..8f5a42ad1f9f 100644 > --- a/arch/x86/kernel/apic/apic_numachip.c > +++ b/arch/x86/kernel/apic/apic_numachip.c > @@ -222,7 +222,6 @@ static const struct apic apic_numachip1 __refconst = { > .probe = numachip1_probe, > .acpi_madt_oem_check = numachip1_acpi_madt_oem_check, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = false, > > .disable_esr = 0, > @@ -259,7 +258,6 @@ static const struct apic apic_numachip2 __refconst = { > .probe = numachip2_probe, > .acpi_madt_oem_check = numachip2_acpi_madt_oem_check, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = false, > > .disable_esr = 0, > diff --git a/arch/x86/kernel/apic/bigsmp_32.c b/arch/x86/kernel/apic/bigsmp_32.c > index 0e5535add4b5..863c3002a574 100644 > --- a/arch/x86/kernel/apic/bigsmp_32.c > +++ b/arch/x86/kernel/apic/bigsmp_32.c > @@ -80,7 +80,6 @@ static struct apic apic_bigsmp __ro_after_init = { > .name = "bigsmp", > .probe = probe_bigsmp, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = false, > > .disable_esr = 1, > diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c > index 9a06df6cdd68..f851ccf1e14f 100644 > --- a/arch/x86/kernel/apic/probe_32.c > +++ b/arch/x86/kernel/apic/probe_32.c > @@ -35,7 +35,6 @@ static struct apic apic_default __ro_after_init = { > .probe = probe_default, > .apic_id_registered = default_apic_id_registered, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = true, > > .disable_esr = 0, > diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c > index affbff65e497..7d15f6c3b718 100644 > --- a/arch/x86/kernel/apic/x2apic_cluster.c > +++ b/arch/x86/kernel/apic/x2apic_cluster.c > @@ -227,7 +227,6 @@ static struct apic apic_x2apic_cluster __ro_after_init = { > .probe = x2apic_cluster_probe, > .acpi_madt_oem_check = x2apic_acpi_madt_oem_check, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = true, > > .disable_esr = 0, > diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c > index 788cdb4ee394..8bb740e22b7d 100644 > --- a/arch/x86/kernel/apic/x2apic_phys.c > +++ b/arch/x86/kernel/apic/x2apic_phys.c > @@ -145,7 +145,6 @@ static struct apic apic_x2apic_phys __ro_after_init = { > .probe = x2apic_phys_probe, > .acpi_madt_oem_check = x2apic_acpi_madt_oem_check, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = false, > > .disable_esr = 0, > diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c > index 7d304ef1a7f5..ae4f0c1a7b43 100644 > --- a/arch/x86/kernel/apic/x2apic_uv_x.c > +++ b/arch/x86/kernel/apic/x2apic_uv_x.c > @@ -805,7 +805,6 @@ static struct apic apic_x2apic_uv_x __ro_after_init = { > .probe = uv_probe, > .acpi_madt_oem_check = uv_acpi_madt_oem_check, > > - .delivery_mode = APIC_DELIVERY_MODE_FIXED, > .dest_mode_logical = false, > > .disable_esr = 0, > diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c > index 4221259a5870..a379501b7a69 100644 > --- a/arch/x86/platform/uv/uv_irq.c > +++ b/arch/x86/platform/uv/uv_irq.c > @@ -35,7 +35,7 @@ static void uv_program_mmr(struct irq_cfg *cfg, struct uv_irq_2_mmr_pnode *info) > mmr_value = 0; > entry = (struct uv_IO_APIC_route_entry *)&mmr_value; > entry->vector = cfg->vector; > - entry->delivery_mode = apic->delivery_mode; > + entry->delivery_mode = APIC_DELIVERY_MODE_FIXED; > entry->dest_mode = apic->dest_mode_logical; > entry->polarity = 0; > entry->trigger = 0; > diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c > index bed3cefdaf19..f5d2ef8572e7 100644 > --- a/drivers/pci/controller/pci-hyperv.c > +++ b/drivers/pci/controller/pci-hyperv.c > @@ -650,13 +650,6 @@ static void hv_arch_irq_unmask(struct irq_data *data) > PCI_FUNC(pdev->devfn); > params->int_target.vector = hv_msi_get_int_vector(data); > > - /* > - * Honoring apic->delivery_mode set to APIC_DELIVERY_MODE_FIXED by > - * setting the HV_DEVICE_INTERRUPT_TARGET_MULTICAST flag results in a > - * spurious interrupt storm. Not doing so does not seem to have a > - * negative effect (yet?). > - */ > - > if (hbus->protocol_version >= PCI_PROTOCOL_VERSION_1_2) { > /* > * PCI_PROTOCOL_VERSION_1_2 supports the VP_SET version of the > > -- > 2.30.2 >
On Thu, Nov 02 2023 at 12:26, Andrew Cooper wrote: > This field is set to APIC_DELIVERY_MODE_FIXED in all cases, and is read > exactly once. Fold the constant in uv_program_mmr() and drop the field. > > Searching for the origin of the stale HyperV comment reveals commit > a31e58e129f7 ("x86/apic: Switch all APICs to Fixed delivery mode") which > notes: > > As a consequence of this change, the apic::irq_delivery_mode field is > now pointless, but this needs to be cleaned up in a separate patch. > > 6 years is long enough for this technical debt to have survived. Guilty as charged. :) Thanks for spotting this and cleaning it up! tglx
diff --git a/arch/x86/include/asm/apic.h b/arch/x86/include/asm/apic.h index 5af4ec1a0f71..841afbd7bfe7 100644 --- a/arch/x86/include/asm/apic.h +++ b/arch/x86/include/asm/apic.h @@ -272,8 +272,6 @@ struct apic { void (*send_IPI_all)(int vector); void (*send_IPI_self)(int vector); - enum apic_delivery_modes delivery_mode; - u32 disable_esr : 1, dest_mode_logical : 1, x2apic_set_max_apicid : 1; diff --git a/arch/x86/kernel/apic/apic_flat_64.c b/arch/x86/kernel/apic/apic_flat_64.c index 032a84e2c3cc..e526b226910b 100644 --- a/arch/x86/kernel/apic/apic_flat_64.c +++ b/arch/x86/kernel/apic/apic_flat_64.c @@ -82,7 +82,6 @@ static struct apic apic_flat __ro_after_init = { .acpi_madt_oem_check = flat_acpi_madt_oem_check, .apic_id_registered = default_apic_id_registered, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = true, .disable_esr = 0, @@ -153,7 +152,6 @@ static struct apic apic_physflat __ro_after_init = { .acpi_madt_oem_check = physflat_acpi_madt_oem_check, .apic_id_registered = default_apic_id_registered, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = false, .disable_esr = 0, diff --git a/arch/x86/kernel/apic/apic_noop.c b/arch/x86/kernel/apic/apic_noop.c index 966d7cf10b95..70e7dfc3cc84 100644 --- a/arch/x86/kernel/apic/apic_noop.c +++ b/arch/x86/kernel/apic/apic_noop.c @@ -45,7 +45,6 @@ static void noop_apic_write(u32 reg, u32 val) struct apic apic_noop __ro_after_init = { .name = "noop", - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = true, .disable_esr = 0, diff --git a/arch/x86/kernel/apic/apic_numachip.c b/arch/x86/kernel/apic/apic_numachip.c index 63f3d7be9dc7..8f5a42ad1f9f 100644 --- a/arch/x86/kernel/apic/apic_numachip.c +++ b/arch/x86/kernel/apic/apic_numachip.c @@ -222,7 +222,6 @@ static const struct apic apic_numachip1 __refconst = { .probe = numachip1_probe, .acpi_madt_oem_check = numachip1_acpi_madt_oem_check, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = false, .disable_esr = 0, @@ -259,7 +258,6 @@ static const struct apic apic_numachip2 __refconst = { .probe = numachip2_probe, .acpi_madt_oem_check = numachip2_acpi_madt_oem_check, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = false, .disable_esr = 0, diff --git a/arch/x86/kernel/apic/bigsmp_32.c b/arch/x86/kernel/apic/bigsmp_32.c index 0e5535add4b5..863c3002a574 100644 --- a/arch/x86/kernel/apic/bigsmp_32.c +++ b/arch/x86/kernel/apic/bigsmp_32.c @@ -80,7 +80,6 @@ static struct apic apic_bigsmp __ro_after_init = { .name = "bigsmp", .probe = probe_bigsmp, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = false, .disable_esr = 1, diff --git a/arch/x86/kernel/apic/probe_32.c b/arch/x86/kernel/apic/probe_32.c index 9a06df6cdd68..f851ccf1e14f 100644 --- a/arch/x86/kernel/apic/probe_32.c +++ b/arch/x86/kernel/apic/probe_32.c @@ -35,7 +35,6 @@ static struct apic apic_default __ro_after_init = { .probe = probe_default, .apic_id_registered = default_apic_id_registered, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = true, .disable_esr = 0, diff --git a/arch/x86/kernel/apic/x2apic_cluster.c b/arch/x86/kernel/apic/x2apic_cluster.c index affbff65e497..7d15f6c3b718 100644 --- a/arch/x86/kernel/apic/x2apic_cluster.c +++ b/arch/x86/kernel/apic/x2apic_cluster.c @@ -227,7 +227,6 @@ static struct apic apic_x2apic_cluster __ro_after_init = { .probe = x2apic_cluster_probe, .acpi_madt_oem_check = x2apic_acpi_madt_oem_check, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = true, .disable_esr = 0, diff --git a/arch/x86/kernel/apic/x2apic_phys.c b/arch/x86/kernel/apic/x2apic_phys.c index 788cdb4ee394..8bb740e22b7d 100644 --- a/arch/x86/kernel/apic/x2apic_phys.c +++ b/arch/x86/kernel/apic/x2apic_phys.c @@ -145,7 +145,6 @@ static struct apic apic_x2apic_phys __ro_after_init = { .probe = x2apic_phys_probe, .acpi_madt_oem_check = x2apic_acpi_madt_oem_check, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = false, .disable_esr = 0, diff --git a/arch/x86/kernel/apic/x2apic_uv_x.c b/arch/x86/kernel/apic/x2apic_uv_x.c index 7d304ef1a7f5..ae4f0c1a7b43 100644 --- a/arch/x86/kernel/apic/x2apic_uv_x.c +++ b/arch/x86/kernel/apic/x2apic_uv_x.c @@ -805,7 +805,6 @@ static struct apic apic_x2apic_uv_x __ro_after_init = { .probe = uv_probe, .acpi_madt_oem_check = uv_acpi_madt_oem_check, - .delivery_mode = APIC_DELIVERY_MODE_FIXED, .dest_mode_logical = false, .disable_esr = 0, diff --git a/arch/x86/platform/uv/uv_irq.c b/arch/x86/platform/uv/uv_irq.c index 4221259a5870..a379501b7a69 100644 --- a/arch/x86/platform/uv/uv_irq.c +++ b/arch/x86/platform/uv/uv_irq.c @@ -35,7 +35,7 @@ static void uv_program_mmr(struct irq_cfg *cfg, struct uv_irq_2_mmr_pnode *info) mmr_value = 0; entry = (struct uv_IO_APIC_route_entry *)&mmr_value; entry->vector = cfg->vector; - entry->delivery_mode = apic->delivery_mode; + entry->delivery_mode = APIC_DELIVERY_MODE_FIXED; entry->dest_mode = apic->dest_mode_logical; entry->polarity = 0; entry->trigger = 0; diff --git a/drivers/pci/controller/pci-hyperv.c b/drivers/pci/controller/pci-hyperv.c index bed3cefdaf19..f5d2ef8572e7 100644 --- a/drivers/pci/controller/pci-hyperv.c +++ b/drivers/pci/controller/pci-hyperv.c @@ -650,13 +650,6 @@ static void hv_arch_irq_unmask(struct irq_data *data) PCI_FUNC(pdev->devfn); params->int_target.vector = hv_msi_get_int_vector(data); - /* - * Honoring apic->delivery_mode set to APIC_DELIVERY_MODE_FIXED by - * setting the HV_DEVICE_INTERRUPT_TARGET_MULTICAST flag results in a - * spurious interrupt storm. Not doing so does not seem to have a - * negative effect (yet?). - */ - if (hbus->protocol_version >= PCI_PROTOCOL_VERSION_1_2) { /* * PCI_PROTOCOL_VERSION_1_2 supports the VP_SET version of the
This field is set to APIC_DELIVERY_MODE_FIXED in all cases, and is read exactly once. Fold the constant in uv_program_mmr() and drop the field. Searching for the origin of the stale HyperV comment reveals commit a31e58e129f7 ("x86/apic: Switch all APICs to Fixed delivery mode") which notes: As a consequence of this change, the apic::irq_delivery_mode field is now pointless, but this needs to be cleaned up in a separate patch. 6 years is long enough for this technical debt to have survived. Signed-off-by: Andrew Cooper <andrew.cooper3@citrix.com> --- arch/x86/include/asm/apic.h | 2 -- 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 ------- 11 files changed, 1 insertion(+), 20 deletions(-)